From 87d9a088d0ea859e737fcb74cf0cf2b747e70dbd Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Wed, 4 Dec 2019 12:09:58 +1000 Subject: [PATCH] config manager str type vault source (#65023) (#65090) * config manager str type vault source * Convert vault text to_text earlier and add tests (cherry picked from commit 7092c196ed0f0e1ee9a53d4040d5ff8c509c05b6) --- .../fragments/config-manager-vault-str.yaml | 2 + lib/ansible/config/manager.py | 9 ++- lib/ansible/plugins/connection/psrp.py | 32 +++++++- lib/ansible/plugins/connection/winrm.py | 16 +++- test/units/config/test_manager.py | 14 ++++ test/units/plugins/connection/test_psrp.py | 6 +- test/units/plugins/connection/test_winrm.py | 81 ++++++++++--------- 7 files changed, 116 insertions(+), 44 deletions(-) create mode 100644 changelogs/fragments/config-manager-vault-str.yaml diff --git a/changelogs/fragments/config-manager-vault-str.yaml b/changelogs/fragments/config-manager-vault-str.yaml new file mode 100644 index 00000000000..40e5c42dde7 --- /dev/null +++ b/changelogs/fragments/config-manager-vault-str.yaml @@ -0,0 +1,2 @@ +bugfixes: +- Fix string parsing of inline vault strings for plugin config variable sources diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py index de146062e34..1731906b311 100644 --- a/lib/ansible/config/manager.py +++ b/lib/ansible/config/manager.py @@ -29,6 +29,7 @@ from ansible.module_utils.six import PY3, string_types from ansible.module_utils.six.moves import configparser from ansible.module_utils.parsing.convert_bool import boolean from ansible.parsing.quoting import unquote +from ansible.parsing.yaml.objects import AnsibleVaultEncryptedUnicode from ansible.utils import py3compat from ansible.utils.path import cleanup_tmp_file, makedirs_safe, unfrackpath @@ -151,7 +152,7 @@ def ensure_type(value, value_type, origin=None): # defaults to string type elif isinstance(value, string_types): - value = unquote(value) + value = unquote(to_text(value, errors='surrogate_or_strict')) if errmsg: raise ValueError('Invalid type provided for "%s": %s' % (errmsg, to_native(value))) @@ -395,7 +396,11 @@ class ConfigManager(object): except UnicodeEncodeError: self.WARNINGS.add(u'value for config entry {0} contains invalid characters, ignoring...'.format(to_text(name))) continue - if temp_value is not None: # only set if env var is defined + if temp_value is not None: # only set if entry is defined in container + # inline vault variables should be converted to a text string + if isinstance(temp_value, AnsibleVaultEncryptedUnicode): + temp_value = to_text(temp_value, errors='surrogate_or_strict') + value = temp_value origin = name diff --git a/lib/ansible/plugins/connection/psrp.py b/lib/ansible/plugins/connection/psrp.py index 8ec648732ff..3545413e34d 100644 --- a/lib/ansible/plugins/connection/psrp.py +++ b/lib/ansible/plugins/connection/psrp.py @@ -21,20 +21,30 @@ options: description: - The hostname or IP address of the remote host. default: inventory_hostname + type: str vars: - name: ansible_host - name: ansible_psrp_host remote_user: description: - The user to log in as. + type: str vars: - name: ansible_user - name: ansible_psrp_user + remote_password: + description: Authentication password for the C(remote_user). Can be supplied as CLI option. + type: str + vars: + - name: ansible_password + - name: ansible_winrm_pass + - name: ansible_winrm_password port: description: - The port for PSRP to connect on the remote target. - Default is C(5986) if I(protocol) is not defined or is C(https), otherwise the port is C(5985). + type: int vars: - name: ansible_port - name: ansible_psrp_port @@ -45,11 +55,13 @@ options: choices: - http - https + type: str vars: - name: ansible_psrp_protocol path: description: - The URI path to connect to. + type: str vars: - name: ansible_psrp_path default: 'wsman' @@ -58,6 +70,7 @@ options: - The authentication protocol to use when authenticating the remote user. - The default, C(negotiate), will attempt to use C(Kerberos) if it is available and fall back to C(NTLM) if it isn't. + type: str vars: - name: ansible_psrp_auth choices: @@ -78,6 +91,7 @@ options: - validate - ignore default: validate + type: str vars: - name: ansible_psrp_cert_validation ca_cert: @@ -85,6 +99,7 @@ options: - The path to a PEM certificate chain to use when validating the server's certificate. - This value is ignored if I(cert_validation) is set to C(ignore). + type: path vars: - name: ansible_psrp_cert_trust_path - name: ansible_psrp_ca_cert @@ -93,6 +108,7 @@ options: description: - The connection timeout for making the request to the remote host. - This is measured in seconds. + type: int vars: - name: ansible_psrp_connection_timeout default: 30 @@ -102,6 +118,7 @@ options: - This value must always be greater than I(operation_timeout). - This option requires pypsrp >= 0.3. - This is measured in seconds. + type: int vars: - name: ansible_psrp_read_timeout default: 30 @@ -109,6 +126,7 @@ options: reconnection_retries: description: - The number of retries on connection errors. + type: int vars: - name: ansible_psrp_reconnection_retries default: 0 @@ -120,6 +138,7 @@ options: - This is measured in seconds. - The C(ansible_psrp_reconnection_backoff) variable was added in Ansible 2.9. + type: int vars: - name: ansible_psrp_connection_backoff - name: ansible_psrp_reconnection_backoff @@ -138,6 +157,7 @@ options: even when running over TLS/HTTPS. - C(never) disables any encryption checks that are in place when running over HTTP and disables any authentication encryption processes. + type: str vars: - name: ansible_psrp_message_encryption choices: @@ -150,6 +170,7 @@ options: - Set the proxy URL to use when connecting to the remote host. vars: - name: ansible_psrp_proxy + type: str ignore_proxy: description: - Will disable any environment proxy settings and connect directly to the @@ -164,11 +185,13 @@ options: certificate_key_pem: description: - The local path to an X509 certificate key to use with certificate auth. + type: path vars: - name: ansible_psrp_certificate_key_pem certificate_pem: description: - The local path to an X509 certificate to use with certificate auth. + type: path vars: - name: ansible_psrp_certificate_pem credssp_auth_mechanism: @@ -176,6 +199,7 @@ options: - The sub authentication mechanism to use with CredSSP auth. - When C(auto), both Kerberos and NTLM is attempted with kerberos being preferred. + type: str choices: - auto - kerberos @@ -208,6 +232,7 @@ options: - Only valid when Kerberos was the negotiated auth or was explicitly set as the authentication. - Ignored when NTLM was the negotiated auth. + type: bool vars: - name: ansible_psrp_negotiate_delegate negotiate_hostname_override: @@ -219,6 +244,7 @@ options: - Only valid when Kerberos was the negotiated auth or was explicitly set as the authentication. - Ignored when NTLM was the negotiated auth. + type: str vars: - name: ansible_psrp_negotiate_hostname_override negotiate_send_cbt: @@ -238,6 +264,7 @@ options: the authentication. - Ignored when NTLM was the negotiated auth. default: WSMAN + type: str vars: - name: ansible_psrp_negotiate_service @@ -247,6 +274,7 @@ options: - Sets the WSMan timeout for each operation. - This is measured in seconds. - This should not exceed the value for C(connection_timeout). + type: int vars: - name: ansible_psrp_operation_timeout default: 20 @@ -255,12 +283,14 @@ options: - Sets the maximum size of each WSMan message sent to the remote host. - This is measured in bytes. - Defaults to C(150KiB) for compatibility with older hosts. + type: int vars: - name: ansible_psrp_max_envelope_size default: 153600 configuration_name: description: - The name of the PowerShell configuration endpoint to connect to. + type: str vars: - name: ansible_psrp_configuration_name default: Microsoft.PowerShell @@ -579,7 +609,7 @@ if ($bytes_read -gt 0) { def _build_kwargs(self): self._psrp_host = self.get_option('remote_addr') self._psrp_user = self.get_option('remote_user') - self._psrp_pass = self._play_context.password + self._psrp_pass = self.get_option('remote_password') protocol = self.get_option('protocol') port = self.get_option('port') diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index 4309a5d88b7..8316c5955f6 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -25,6 +25,7 @@ DOCUMENTATION = """ vars: - name: ansible_host - name: ansible_winrm_host + type: str remote_user: keywords: - name: user @@ -34,6 +35,14 @@ DOCUMENTATION = """ vars: - name: ansible_user - name: ansible_winrm_user + type: str + remote_password: + description: Authentication password for the C(remote_user). Can be supplied as CLI option. + vars: + - name: ansible_password + - name: ansible_winrm_pass + - name: ansible_winrm_password + type: str port: description: - port for winrm to connect on remote target @@ -53,11 +62,13 @@ DOCUMENTATION = """ choices: [http, https] vars: - name: ansible_winrm_scheme + type: str path: description: URI path to connect to default: '/wsman' vars: - name: ansible_winrm_path + type: str transport: description: - List of winrm transports to attempt to to use (ssl, plaintext, kerberos, etc) @@ -71,6 +82,7 @@ DOCUMENTATION = """ default: kinit vars: - name: ansible_winrm_kinit_cmd + type: str kerberos_mode: description: - kerberos usage mode. @@ -83,6 +95,7 @@ DOCUMENTATION = """ choices: [managed, manual] vars: - name: ansible_winrm_kinit_mode + type: str connection_timeout: description: - Sets the operation and read timeout settings for the WinRM @@ -94,6 +107,7 @@ DOCUMENTATION = """ pywinrm. vars: - name: ansible_winrm_connection_timeout + type: int """ import base64 @@ -205,7 +219,7 @@ class Connection(ConnectionBase): # starting the WinRM connection self._winrm_host = self.get_option('remote_addr') self._winrm_user = self.get_option('remote_user') - self._winrm_pass = self._play_context.password + self._winrm_pass = self.get_option('remote_password') self._winrm_port = self.get_option('port') diff --git a/test/units/config/test_manager.py b/test/units/config/test_manager.py index c6ad36609db..0a0a71af97f 100644 --- a/test/units/config/test_manager.py +++ b/test/units/config/test_manager.py @@ -13,6 +13,7 @@ import pytest from ansible.config.manager import ConfigManager, Setting, ensure_type, resolve_path, get_config_type from ansible.errors import AnsibleOptionsError, AnsibleError from ansible.module_utils.six import integer_types, string_types +from ansible.parsing.yaml.objects import AnsibleVaultEncryptedUnicode curdir = os.path.dirname(__file__) cfg_file = os.path.join(curdir, 'test.cfg') @@ -117,3 +118,16 @@ class TestConfigManager: self.manager._read_config_yaml_file(os.path.join(curdir, 'test_non_existent.yml')) assert "Missing base YAML definition file (bad install?)" in str(exec_info.value) + + def test_entry_as_vault_var(self): + class MockVault: + + def decrypt(self, value): + return value + + vault_var = AnsibleVaultEncryptedUnicode(b"vault text") + vault_var.vault = MockVault() + + actual_value, actual_origin = self.manager._loop_entries({'name': vault_var}, [{'name': 'name'}]) + assert actual_value == "vault text" + assert actual_origin == "name" diff --git a/test/units/plugins/connection/test_psrp.py b/test/units/plugins/connection/test_psrp.py index c054bef5abf..f64167518ee 100644 --- a/test/units/plugins/connection/test_psrp.py +++ b/test/units/plugins/connection/test_psrp.py @@ -82,7 +82,7 @@ class TestConnectionPSRP(object): 'server': 'inventory_hostname', 'port': 5986, 'username': None, - 'password': '', + 'password': None, 'ssl': True, 'path': 'wsman', 'auth': 'negotiate', @@ -109,7 +109,7 @@ class TestConnectionPSRP(object): '_psrp_max_envelope_size': 153600, '_psrp_ignore_proxy': False, '_psrp_operation_timeout': 20, - '_psrp_pass': '', + '_psrp_pass': None, '_psrp_path': 'wsman', '_psrp_port': 5986, '_psrp_proxy': None, @@ -157,7 +157,7 @@ class TestConnectionPSRP(object): 'server': 'inventory_hostname', 'port': 5986, 'username': None, - 'password': '', + 'password': None, 'ssl': True, 'path': 'wsman', 'auth': 'negotiate', diff --git a/test/units/plugins/connection/test_winrm.py b/test/units/plugins/connection/test_winrm.py index fd40a3f2f4b..67bfd9ae460 100644 --- a/test/units/plugins/connection/test_winrm.py +++ b/test/units/plugins/connection/test_winrm.py @@ -25,7 +25,6 @@ class TestConnectionWinRM(object): OPTIONS_DATA = ( # default options ( - {}, {'_extras': {}}, {}, { @@ -33,8 +32,8 @@ class TestConnectionWinRM(object): '_kinit_cmd': 'kinit', '_winrm_connection_timeout': None, '_winrm_host': 'inventory_hostname', - '_winrm_kwargs': {'username': None, 'password': ''}, - '_winrm_pass': '', + '_winrm_kwargs': {'username': None, 'password': None}, + '_winrm_pass': None, '_winrm_path': '/wsman', '_winrm_port': 5986, '_winrm_scheme': 'https', @@ -45,11 +44,10 @@ class TestConnectionWinRM(object): ), # http through port ( - {}, {'_extras': {}, 'ansible_port': 5985}, {}, { - '_winrm_kwargs': {'username': None, 'password': ''}, + '_winrm_kwargs': {'username': None, 'password': None}, '_winrm_port': 5985, '_winrm_scheme': 'http', '_winrm_transport': ['plaintext'], @@ -58,15 +56,14 @@ class TestConnectionWinRM(object): ), # kerberos user with kerb present ( - {}, {'_extras': {}, 'ansible_user': 'user@domain.com'}, {}, { '_kerb_managed': False, '_kinit_cmd': 'kinit', '_winrm_kwargs': {'username': 'user@domain.com', - 'password': ''}, - '_winrm_pass': '', + 'password': None}, + '_winrm_pass': None, '_winrm_transport': ['kerberos', 'ssl'], '_winrm_user': 'user@domain.com' }, @@ -74,15 +71,14 @@ class TestConnectionWinRM(object): ), # kerberos user without kerb present ( - {}, {'_extras': {}, 'ansible_user': 'user@domain.com'}, {}, { '_kerb_managed': False, '_kinit_cmd': 'kinit', '_winrm_kwargs': {'username': 'user@domain.com', - 'password': ''}, - '_winrm_pass': '', + 'password': None}, + '_winrm_pass': None, '_winrm_transport': ['ssl'], '_winrm_user': 'user@domain.com' }, @@ -90,9 +86,8 @@ class TestConnectionWinRM(object): ), # kerberos user with managed ticket (implicit) ( - {'password': 'pass'}, {'_extras': {}, 'ansible_user': 'user@domain.com'}, - {}, + {'remote_password': 'pass'}, { '_kerb_managed': True, '_kinit_cmd': 'kinit', @@ -106,10 +101,9 @@ class TestConnectionWinRM(object): ), # kerb with managed ticket (explicit) ( - {'password': 'pass'}, {'_extras': {}, 'ansible_user': 'user@domain.com', 'ansible_winrm_kinit_mode': 'managed'}, - {}, + {'password': 'pass'}, { '_kerb_managed': True, }, @@ -117,10 +111,9 @@ class TestConnectionWinRM(object): ), # kerb with unmanaged ticket (explicit)) ( - {'password': 'pass'}, {'_extras': {}, 'ansible_user': 'user@domain.com', 'ansible_winrm_kinit_mode': 'manual'}, - {}, + {'password': 'pass'}, { '_kerb_managed': False, }, @@ -128,40 +121,37 @@ class TestConnectionWinRM(object): ), # transport override (single) ( - {}, {'_extras': {}, 'ansible_user': 'user@domain.com', 'ansible_winrm_transport': 'ntlm'}, {}, { '_winrm_kwargs': {'username': 'user@domain.com', - 'password': ''}, - '_winrm_pass': '', + 'password': None}, + '_winrm_pass': None, '_winrm_transport': ['ntlm'], }, False ), # transport override (list) ( - {}, {'_extras': {}, 'ansible_user': 'user@domain.com', 'ansible_winrm_transport': ['ntlm', 'certificate']}, {}, { '_winrm_kwargs': {'username': 'user@domain.com', - 'password': ''}, - '_winrm_pass': '', + 'password': None}, + '_winrm_pass': None, '_winrm_transport': ['ntlm', 'certificate'], }, False ), # winrm extras ( - {}, {'_extras': {'ansible_winrm_server_cert_validation': 'ignore', 'ansible_winrm_service': 'WSMAN'}}, {}, { - '_winrm_kwargs': {'username': None, 'password': '', + '_winrm_kwargs': {'username': None, 'password': None, 'server_cert_validation': 'ignore', 'service': 'WSMAN'}, }, @@ -169,7 +159,6 @@ class TestConnectionWinRM(object): ), # direct override ( - {}, {'_extras': {}, 'ansible_winrm_connection_timeout': 5}, {'connection_timeout': 10}, { @@ -177,29 +166,47 @@ class TestConnectionWinRM(object): }, False ), - # user comes from option not play context + # password as ansible_password ( - {'username': 'user1'}, - {'_extras': {}, 'ansible_user': 'user2'}, + {'_extras': {}, 'ansible_password': 'pass'}, {}, { - '_winrm_user': 'user2', - '_winrm_kwargs': {'username': 'user2', 'password': ''} + '_winrm_pass': 'pass', + '_winrm_kwargs': {'username': None, 'password': 'pass'} }, False - ) + ), + # password as ansible_winrm_pass + ( + {'_extras': {}, 'ansible_winrm_pass': 'pass'}, + {}, + { + '_winrm_pass': 'pass', + '_winrm_kwargs': {'username': None, 'password': 'pass'} + }, + False + ), + + # password as ansible_winrm_password + ( + {'_extras': {}, 'ansible_winrm_password': 'pass'}, + {}, + { + '_winrm_pass': 'pass', + '_winrm_kwargs': {'username': None, 'password': 'pass'} + }, + False + ), ) # pylint bug: https://github.com/PyCQA/pylint/issues/511 # pylint: disable=undefined-variable - @pytest.mark.parametrize('play, options, direct, expected, kerb', - ((p, o, d, e, k) for p, o, d, e, k in OPTIONS_DATA)) - def test_set_options(self, play, options, direct, expected, kerb): + @pytest.mark.parametrize('options, direct, expected, kerb', + ((o, d, e, k) for o, d, e, k in OPTIONS_DATA)) + def test_set_options(self, options, direct, expected, kerb): winrm.HAVE_KERBEROS = kerb pc = PlayContext() - for attr, value in play.items(): - setattr(pc, attr, value) new_stdin = StringIO() conn = connection_loader.get('winrm', pc, new_stdin)