From a1dddce6fda47e5b09e0aa0f94b4f5420ac6fcb9 Mon Sep 17 00:00:00 2001 From: Claudiu Popescu Date: Thu, 22 Nov 2018 16:27:31 +0200 Subject: [PATCH 01/55] Fixes icinga2 certs path for newer versions 2.8+ Fixes #45867 --- salt/modules/icinga2.py | 28 ++++++++------------------- salt/states/icinga2.py | 13 +++++++------ salt/utils/icinga2.py | 42 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 26 deletions(-) create mode 100644 salt/utils/icinga2.py diff --git a/salt/modules/icinga2.py b/salt/modules/icinga2.py index 208bae0dd979..1e6971654be2 100644 --- a/salt/modules/icinga2.py +++ b/salt/modules/icinga2.py @@ -10,11 +10,11 @@ # Import python libs from __future__ import absolute_import, print_function, unicode_literals import logging -import subprocess # Import Salt libs import salt.utils.path import salt.utils.platform +from salt.utils.icinga2 import get_certs_path, execute log = logging.getLogger(__name__) @@ -32,18 +32,6 @@ def __virtual__(): return (False, 'Icinga2 not installed.') -def _execute(cmd, ret_code=False): - process = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE) - if ret_code: - return process.wait() - output, error = process.communicate() - if output: - log.debug(output) - return output - log.debug(error) - return error - - def generate_ticket(domain): ''' Generate and save an icinga2 ticket. @@ -58,7 +46,7 @@ def generate_ticket(domain): salt '*' icinga2.generate_ticket domain.tld ''' - result = _execute(["icinga2", "pki", "ticket", "--cn", domain]) + result = execute(["icinga2", "pki", "ticket", "--cn", domain]) return result @@ -76,7 +64,7 @@ def generate_cert(domain): salt '*' icinga2.generate_cert domain.tld ''' - result = _execute(["icinga2", "pki", "new-cert", "--cn", domain, "--key", "/etc/icinga2/pki/{0}.key".format(domain), "--cert", "/etc/icinga2/pki/{0}.crt".format(domain)], ret_code=True) + result = execute(["icinga2", "pki", "new-cert", "--cn", domain, "--key", "{0}{1}.key".format(get_certs_path(), domain), "--cert", "{0}{1}.crt".format(get_certs_path(), domain)], ret_code=True) return result @@ -94,8 +82,8 @@ def save_cert(domain, master): salt '*' icinga2.save_cert domain.tld master.domain.tld ''' - result = _execute(["icinga2", "pki", "save-cert", "--key", "/etc/icinga2/pki/{0}.key".format(domain), "--cert", "/etc/icinga2/pki/{0}.cert".format(domain), "--trustedcert", - "/etc/icinga2/pki/trusted-master.crt", "--host", master], ret_code=True) + result = execute(["icinga2", "pki", "save-cert", "--key", "{0}{1}.key".format(get_certs_path(), domain), "--cert", "{0}{1}.cert".format(get_certs_path(), domain), "--trustedcert", + "{0}trusted-master.crt".format(get_certs_path()), "--host", master], ret_code=True) return result @@ -114,8 +102,8 @@ def request_cert(domain, master, ticket, port): salt '*' icinga2.request_cert domain.tld master.domain.tld TICKET_ID ''' - result = _execute(["icinga2", "pki", "request", "--host", master, "--port", port, "--ticket", ticket, "--key", "/etc/icinga2/pki/{0}.key".format(domain), "--cert", - "/etc/icinga2/pki/{0}.crt".format(domain), "--trustedcert", "/etc/icinga2/pki/trusted-master.crt", "--ca", "/etc/icinga2/pki/ca.crt"], ret_code=True) + result = execute(["icinga2", "pki", "request", "--host", master, "--port", port, "--ticket", ticket, "--key", "{0}{1}.key".format(get_certs_path(), domain), "--cert", + "{0}{1}.crt".format(get_certs_path(), domain), "--trustedcert", "{0}trusted-master.crt".format{get_certs_path()), "--ca", "{0}ca.crt".format(get_certs_path())], ret_code=True) return result @@ -134,6 +122,6 @@ def node_setup(domain, master, ticket): salt '*' icinga2.node_setup domain.tld master.domain.tld TICKET_ID ''' - result = _execute(["icinga2", "node", "setup", "--ticket", ticket, "--endpoint", master, "--zone", domain, "--master_host", master, "--trustedcert", "/etc/icinga2/pki/trusted-master.crt"], + result = execute(["icinga2", "node", "setup", "--ticket", ticket, "--endpoint", master, "--zone", domain, "--master_host", master, "--trustedcert", "{0}trusted-master.crt".format(get_certs_path())], ret_code=True) return result diff --git a/salt/states/icinga2.py b/salt/states/icinga2.py index 64ae25007101..8544da579819 100644 --- a/salt/states/icinga2.py +++ b/salt/states/icinga2.py @@ -27,6 +27,7 @@ from salt.ext import six import salt.utils.files import salt.utils.stringutils +from salt.utils.icinga2 import get_certs_path def __virtual__(): @@ -140,8 +141,8 @@ def generate_cert(name): 'changes': {}, 'result': True, 'comment': ''} - cert = "/etc/icinga2/pki/{0}.crt".format(name) - key = "/etc/icinga2/pki/{0}.key".format(name) + cert = "{0}{1}.crt".format(get_certs_path(), name) + key = "{0}{1}.key".format(get_certs_path(), name) # Checking if execution is needed. if os.path.isfile(cert) and os.path.isfile(key): @@ -175,7 +176,7 @@ def save_cert(name, master): 'changes': {}, 'result': True, 'comment': ''} - cert = "/etc/icinga2/pki/trusted-master.crt" + cert = "{0}trusted-master.crt".format(get_certs_path()) # Checking if execution is needed. if os.path.isfile(cert): @@ -214,7 +215,7 @@ def request_cert(name, master, ticket, port="5665"): 'changes': {}, 'result': True, 'comment': ''} - cert = "/etc/icinga2/pki/ca.crt" + cert = "{0}ca.crt".format(get_certs_path()) # Checking if execution is needed. if os.path.isfile(cert): @@ -254,8 +255,8 @@ def node_setup(name, master, ticket): 'changes': {}, 'result': True, 'comment': ''} - cert = "/etc/icinga2/pki/{0}.crt.orig".format(name) - key = "/etc/icinga2/pki/{0}.key.orig".format(name) + cert = "{0}{1}.crt.orig".format(get_certs_path(), name) + key = "{0}{1}.key.orig".format(get_certs_path(), name) # Checking if execution is needed. if os.path.isfile(cert) and os.path.isfile(cert): diff --git a/salt/utils/icinga2.py b/salt/utils/icinga2.py new file mode 100644 index 000000000000..a3fb706ddd6b --- /dev/null +++ b/salt/utils/icinga2.py @@ -0,0 +1,42 @@ +# -*- coding: utf-8 -*- +''' +Icinga2 Common Utils +================= + +This module provides common functionality for icinga2 module and state. + +.. versionadded:: 2018.8.3 +''' + +# Import python libs +import logging +import subprocess +import re + +# Import Salt libs +import salt.utils.path + +log = logging.getLogger(__name__) + + +def execute(cmd, ret_code=False): + process = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE) + if ret_code: + return process.wait() + output, error = process.communicate() + if output: + log.debug(output) + return output + log.debug(error) + return error + + +def get_certs_path(): + icinga2_output = execute([salt.utils.path.which('icinga2'), "--version"]) + version = re.search('r\d+\.\d+', icinga2_output).group(0) + # Return new certs path for icinga2 >= 2.8 + if int(version.split('.')[1]) >= 8: + return '/var/lib/icinga2/certs/' + # Keep backwords compatibility with older icinga2 + return '/etc/icinga2/pki/' + From 47bfba1018db9b6faa2f42da9fa621b92e337527 Mon Sep 17 00:00:00 2001 From: Claudiu Popescu Date: Fri, 23 Nov 2018 16:11:01 +0200 Subject: [PATCH 02/55] Migrated to cmd.run_all and tested on working srv --- salt/modules/icinga2.py | 18 +++++++++--------- salt/states/icinga2.py | 17 +++++++++-------- salt/utils/icinga2.py | 17 ++--------------- 3 files changed, 20 insertions(+), 32 deletions(-) diff --git a/salt/modules/icinga2.py b/salt/modules/icinga2.py index 1e6971654be2..66cb8841660c 100644 --- a/salt/modules/icinga2.py +++ b/salt/modules/icinga2.py @@ -14,7 +14,7 @@ # Import Salt libs import salt.utils.path import salt.utils.platform -from salt.utils.icinga2 import get_certs_path, execute +from salt.utils.icinga2 import get_certs_path log = logging.getLogger(__name__) @@ -46,7 +46,7 @@ def generate_ticket(domain): salt '*' icinga2.generate_ticket domain.tld ''' - result = execute(["icinga2", "pki", "ticket", "--cn", domain]) + result = __salt__['cmd.run_all'](["icinga2", "pki", "ticket", "--cn", domain], python_shell=False) return result @@ -64,7 +64,7 @@ def generate_cert(domain): salt '*' icinga2.generate_cert domain.tld ''' - result = execute(["icinga2", "pki", "new-cert", "--cn", domain, "--key", "{0}{1}.key".format(get_certs_path(), domain), "--cert", "{0}{1}.crt".format(get_certs_path(), domain)], ret_code=True) + result = __salt__['cmd.run_all'](["icinga2", "pki", "new-cert", "--cn", domain, "--key", "{0}{1}.key".format(get_certs_path(), domain), "--cert", "{0}{1}.crt".format(get_certs_path(), domain)], python_shell=False) return result @@ -82,8 +82,8 @@ def save_cert(domain, master): salt '*' icinga2.save_cert domain.tld master.domain.tld ''' - result = execute(["icinga2", "pki", "save-cert", "--key", "{0}{1}.key".format(get_certs_path(), domain), "--cert", "{0}{1}.cert".format(get_certs_path(), domain), "--trustedcert", - "{0}trusted-master.crt".format(get_certs_path()), "--host", master], ret_code=True) + result = __salt__['cmd.run_all'](["icinga2", "pki", "save-cert", "--key", "{0}{1}.key".format(get_certs_path(), domain), "--cert", "{0}{1}.cert".format(get_certs_path(), domain), "--trustedcert", + "{0}trusted-master.crt".format(get_certs_path()), "--host", master], python_shell=False) return result @@ -102,8 +102,8 @@ def request_cert(domain, master, ticket, port): salt '*' icinga2.request_cert domain.tld master.domain.tld TICKET_ID ''' - result = execute(["icinga2", "pki", "request", "--host", master, "--port", port, "--ticket", ticket, "--key", "{0}{1}.key".format(get_certs_path(), domain), "--cert", - "{0}{1}.crt".format(get_certs_path(), domain), "--trustedcert", "{0}trusted-master.crt".format{get_certs_path()), "--ca", "{0}ca.crt".format(get_certs_path())], ret_code=True) + result = __salt__['cmd.run_all'](["icinga2", "pki", "request", "--host", master, "--port", port, "--ticket", ticket, "--key", "{0}{1}.key".format(get_certs_path(), domain), "--cert", + "{0}{1}.crt".format(get_certs_path(), domain), "--trustedcert", "{0}trusted-master.crt".format(get_certs_path()), "--ca", "{0}ca.crt".format(get_certs_path())], python_shell=False) return result @@ -122,6 +122,6 @@ def node_setup(domain, master, ticket): salt '*' icinga2.node_setup domain.tld master.domain.tld TICKET_ID ''' - result = execute(["icinga2", "node", "setup", "--ticket", ticket, "--endpoint", master, "--zone", domain, "--master_host", master, "--trustedcert", "{0}trusted-master.crt".format(get_certs_path())], - ret_code=True) + result = __salt__['cmd.run_all'](["icinga2", "node", "setup", "--ticket", ticket, "--endpoint", master, "--zone", domain, "--master_host", master, "--trustedcert", "{0}trusted-master.crt".format(get_certs_path())], + python_shell=False) return result diff --git a/salt/states/icinga2.py b/salt/states/icinga2.py index 8544da579819..e36f2e6638e6 100644 --- a/salt/states/icinga2.py +++ b/salt/states/icinga2.py @@ -104,8 +104,9 @@ def generate_ticket(name, output=None, grain=None, key=None, overwrite=True): return ret # Executing the command. - ticket = __salt__['icinga2.generate_ticket'](name).strip() - if ticket: + ticket_res = __salt__['icinga2.generate_ticket'](name) + ticket = ticket_res['stdout'] + if not ticket_res['retcode']: ret['comment'] = six.text_type(ticket) if output == 'grain': @@ -155,7 +156,7 @@ def generate_cert(name): # Executing the command. cert_save = __salt__['icinga2.generate_cert'](name) - if not cert_save: + if not cert_save['retcode']: ret['comment'] = "Certificate and key generated" ret['changes']['cert'] = "Executed. Certificate saved: {0}".format(cert) ret['changes']['key'] = "Executed. Key saved: {0}".format(key) @@ -189,7 +190,7 @@ def save_cert(name, master): # Executing the command. cert_save = __salt__['icinga2.save_cert'](name, master) - if not cert_save: + if not cert_save['retcode']: ret['comment'] = "Certificate for icinga2 master saved" ret['changes']['cert'] = "Executed. Certificate saved: {0}".format(cert) return ret @@ -228,12 +229,12 @@ def request_cert(name, master, ticket, port="5665"): # Executing the command. cert_request = __salt__['icinga2.request_cert'](name, master, ticket, port) - if not cert_request: + if not cert_request['retcode']: ret['comment'] = "Certificate request from icinga2 master executed" ret['changes']['cert'] = "Executed. Certificate requested: {0}".format(cert) return ret - ret['comment'] = "FAILED. Certificate requested failed with exit code: {0}".format(cert_request) + ret['comment'] = "FAILED. Certificate requested failed with output: {0}".format(cert_request['stdout']) ret['result'] = False return ret @@ -269,11 +270,11 @@ def node_setup(name, master, ticket): # Executing the command. node_setup = __salt__['icinga2.node_setup'](name, master, ticket) - if not node_setup: + if not node_setup['retcode']: ret['comment'] = "Node setup executed." ret['changes']['cert'] = "Node setup finished successfully." return ret - ret['comment'] = "FAILED. Node setup failed with exit code: {0}".format(node_setup) + ret['comment'] = "FAILED. Node setup failed with outpu: {0}".format(node_setup['stdout']) ret['result'] = False return ret diff --git a/salt/utils/icinga2.py b/salt/utils/icinga2.py index a3fb706ddd6b..27b15e3cc780 100644 --- a/salt/utils/icinga2.py +++ b/salt/utils/icinga2.py @@ -10,7 +10,6 @@ # Import python libs import logging -import subprocess import re # Import Salt libs @@ -19,21 +18,9 @@ log = logging.getLogger(__name__) -def execute(cmd, ret_code=False): - process = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE) - if ret_code: - return process.wait() - output, error = process.communicate() - if output: - log.debug(output) - return output - log.debug(error) - return error - - def get_certs_path(): - icinga2_output = execute([salt.utils.path.which('icinga2'), "--version"]) - version = re.search('r\d+\.\d+', icinga2_output).group(0) + icinga2_output = __salt__['cmd.run_all']([salt.utils.path.which('icinga2'), "--version"], python_shell=False) + version = re.search('r\d+\.\d+', icinga2_output['stdout']).group(0) # Return new certs path for icinga2 >= 2.8 if int(version.split('.')[1]) >= 8: return '/var/lib/icinga2/certs/' From 79f6b426ffe2fb996f5ec9737a3608f7dfa6100e Mon Sep 17 00:00:00 2001 From: Lex Vella Date: Tue, 27 Nov 2018 15:08:27 -0500 Subject: [PATCH 03/55] Check file_mode in file.directory and _check_directory --- salt/states/file.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/salt/states/file.py b/salt/states/file.py index 117f6d92abbe..e571cec2920b 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -677,6 +677,7 @@ def _check_directory(name, group=None, recurse=False, mode=None, + file_mode=None, clean=False, require=False, exclude_pat=None, @@ -712,6 +713,7 @@ def _check_directory(name, if check_files: for fname in files: fchange = {} + mode = file_mode path = os.path.join(root, fname) stats = __salt__['file.stats']( path, None, follow_symlinks @@ -720,6 +722,8 @@ def _check_directory(name, fchange['user'] = user if group is not None and group != stats.get('group'): fchange['group'] = group + if mode is not None and mode != stats.get('mode'): + fchange['mode'] = mode if fchange: changes[path] = fchange if check_dirs: @@ -3119,8 +3123,8 @@ def directory(name, win_perms_reset=win_perms_reset) else: presult, pcomment, pchanges = _check_directory( - name, user, group, recurse or [], dir_mode, clean, require, - exclude_pat, max_depth, follow_symlinks) + name, user, group, recurse or [], dir_mode, file_mode, clean, + require, exclude_pat, max_depth, follow_symlinks) if pchanges: ret['pchanges'].update(pchanges) From 31878d5aaf894130b9f923ca970e4751a680d4f1 Mon Sep 17 00:00:00 2001 From: Claudiu Popescu Date: Wed, 28 Nov 2018 23:20:51 +0200 Subject: [PATCH 04/55] Pylint fixes --- salt/utils/icinga2.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/salt/utils/icinga2.py b/salt/utils/icinga2.py index 27b15e3cc780..b5b213d06be7 100644 --- a/salt/utils/icinga2.py +++ b/salt/utils/icinga2.py @@ -7,8 +7,8 @@ .. versionadded:: 2018.8.3 ''' - # Import python libs +from __future__ import absolute_import, print_function, unicode_literals import logging import re @@ -20,10 +20,9 @@ def get_certs_path(): icinga2_output = __salt__['cmd.run_all']([salt.utils.path.which('icinga2'), "--version"], python_shell=False) - version = re.search('r\d+\.\d+', icinga2_output['stdout']).group(0) + version = re.search(r'r\d+\.\d+', icinga2_output['stdout']).group(0) # Return new certs path for icinga2 >= 2.8 if int(version.split('.')[1]) >= 8: return '/var/lib/icinga2/certs/' # Keep backwords compatibility with older icinga2 return '/etc/icinga2/pki/' - From a394cd76f6d4157ac3e665b1980473dd41fb021d Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Thu, 29 Nov 2018 09:29:23 -0600 Subject: [PATCH 05/55] Update file integration tests to use decorator for tempdirs --- tests/integration/states/test_file.py | 53 ++++++++++----------------- 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index 3da51ef97b56..d48ec39bbf75 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -13,7 +13,6 @@ import sys import shutil import stat -import tempfile import textwrap import filecmp @@ -501,15 +500,7 @@ def test_managed_contents(self): managed_files = {} state_keys = {} for typ in ('bool', 'str', 'int', 'float', 'list', 'dict'): - fd_, managed_files[typ] = tempfile.mkstemp() - - # Release the handle so they can be removed in Windows - try: - os.close(fd_) - except OSError as exc: - if exc.errno != errno.EBADF: - raise exc - + managed_files[typ] = salt.utils.files.mkstemp() state_keys[typ] = 'file_|-{0} file_|-{1}_|-managed'.format(typ, managed_files[typ]) try: with salt.utils.files.fopen(state_file, 'w') as fd_: @@ -986,7 +977,8 @@ def test_test_directory_clean_exclude(self, base_dir): self.assertIn(strayfile2, comment) self.assertNotIn(keepfile, comment) - def test_directory_clean_require_in(self): + @with_tempdir() + def test_directory_clean_require_in(self, name): ''' file.directory test with clean=True and require_in file ''' @@ -994,33 +986,31 @@ def test_directory_clean_require_in(self): state_filename = state_name + '.sls' state_file = os.path.join(BASE_FILES, state_filename) - directory = tempfile.mkdtemp() - self.addCleanup(lambda: shutil.rmtree(directory)) - - wrong_file = os.path.join(directory, "wrong") + wrong_file = os.path.join(name, "wrong") with salt.utils.files.fopen(wrong_file, "w") as fp: fp.write("foo") - good_file = os.path.join(directory, "bar") + good_file = os.path.join(name, "bar") with salt.utils.files.fopen(state_file, 'w') as fp: self.addCleanup(lambda: os.remove(state_file)) fp.write(textwrap.dedent('''\ some_dir: file.directory: - - name: {directory} + - name: {name} - clean: true {good_file}: file.managed: - require_in: - file: some_dir - '''.format(directory=directory, good_file=good_file))) + '''.format(name=name, good_file=good_file))) ret = self.run_function('state.sls', [state_name]) self.assertTrue(os.path.exists(good_file)) self.assertFalse(os.path.exists(wrong_file)) - def test_directory_clean_require_in_with_id(self): + @with_tempdir() + def test_directory_clean_require_in_with_id(self, name): ''' file.directory test with clean=True and require_in file with an ID different from the file name @@ -1029,20 +1019,17 @@ def test_directory_clean_require_in_with_id(self): state_filename = state_name + '.sls' state_file = os.path.join(BASE_FILES, state_filename) - directory = tempfile.mkdtemp() - self.addCleanup(lambda: shutil.rmtree(directory)) - - wrong_file = os.path.join(directory, "wrong") + wrong_file = os.path.join(name, "wrong") with salt.utils.files.fopen(wrong_file, "w") as fp: fp.write("foo") - good_file = os.path.join(directory, "bar") + good_file = os.path.join(name, "bar") with salt.utils.files.fopen(state_file, 'w') as fp: self.addCleanup(lambda: os.remove(state_file)) fp.write(textwrap.dedent('''\ some_dir: file.directory: - - name: {directory} + - name: {name} - clean: true some_file: @@ -1050,13 +1037,14 @@ def test_directory_clean_require_in_with_id(self): - name: {good_file} - require_in: - file: some_dir - '''.format(directory=directory, good_file=good_file))) + '''.format(name=name, good_file=good_file))) ret = self.run_function('state.sls', [state_name]) self.assertTrue(os.path.exists(good_file)) self.assertFalse(os.path.exists(wrong_file)) - def test_directory_clean_require_with_name(self): + @with_tempdir() + def test_directory_clean_require_with_name(self, name): ''' file.directory test with clean=True and require with a file state relatively to the state's name, not its ID. @@ -1065,20 +1053,17 @@ def test_directory_clean_require_with_name(self): state_filename = state_name + '.sls' state_file = os.path.join(BASE_FILES, state_filename) - directory = tempfile.mkdtemp() - self.addCleanup(lambda: shutil.rmtree(directory)) - - wrong_file = os.path.join(directory, "wrong") + wrong_file = os.path.join(name, "wrong") with salt.utils.files.fopen(wrong_file, "w") as fp: fp.write("foo") - good_file = os.path.join(directory, "bar") + good_file = os.path.join(name, "bar") with salt.utils.files.fopen(state_file, 'w') as fp: self.addCleanup(lambda: os.remove(state_file)) fp.write(textwrap.dedent('''\ some_dir: file.directory: - - name: {directory} + - name: {name} - clean: true - require: # This requirement refers to the name of the following @@ -1088,7 +1073,7 @@ def test_directory_clean_require_with_name(self): some_file: file.managed: - name: {good_file} - '''.format(directory=directory, good_file=good_file))) + '''.format(name=name, good_file=good_file))) ret = self.run_function('state.sls', [state_name]) self.assertTrue(os.path.exists(good_file)) From be14517c5c95e1d73c38f0bd76785e4ef4b1ef07 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Thu, 29 Nov 2018 14:48:07 -0600 Subject: [PATCH 06/55] Make x509 tests use tempfiles instead of hard-coding them This keeps us from needing to manually clean them up in the tearDown --- .../files/file/base/issue-49008.sls | 8 +- .../integration/files/file/base/x509_test.crt | 43 +++++++++ tests/integration/states/test_x509.py | 87 +++++++++---------- 3 files changed, 89 insertions(+), 49 deletions(-) create mode 100644 tests/integration/files/file/base/x509_test.crt diff --git a/tests/integration/files/file/base/issue-49008.sls b/tests/integration/files/file/base/issue-49008.sls index 7f9019db35b9..374fbd7f213b 100644 --- a/tests/integration/files/file/base/issue-49008.sls +++ b/tests/integration/files/file/base/issue-49008.sls @@ -1,6 +1,6 @@ -/test-ca-49008.crt: +{{ pillar['crtfile'] }}: x509.certificate_managed: - - signing_private_key: /test-ca-49008.key + - signing_private_key: {{ pillar['keyfile'] }} - CN: testy-mctest - basicConstraints: "critical CA:true" - keyUsage: "critical cRLSign, keyCertSign" @@ -10,9 +10,9 @@ - days_remaining: 0 - backup: True - watch: - - x509: /test-ca-49008.key + - x509: {{ pillar['keyfile'] }} -/test-ca-49008.key: +{{ pillar['keyfile'] }}: x509.private_key_managed: - bits: 4096 - backup: True diff --git a/tests/integration/files/file/base/x509_test.crt b/tests/integration/files/file/base/x509_test.crt new file mode 100644 index 000000000000..5855213cbab5 --- /dev/null +++ b/tests/integration/files/file/base/x509_test.crt @@ -0,0 +1,43 @@ +-----BEGIN CERTIFICATE----- +MIIHjzCCBnegAwIBAgIIBzL2FMQfSVYwDQYJKoZIhvcNAQELBQAwVDELMAkGA1UE +BhMCVVMxHjAcBgNVBAoTFUdvb2dsZSBUcnVzdCBTZXJ2aWNlczElMCMGA1UEAxMc +R29vZ2xlIEludGVybmV0IEF1dGhvcml0eSBHMzAeFw0xODA3MjQxNjA4MjVaFw0x +ODEwMDIxNjAwMDBaMGYxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlh +MRYwFAYDVQQHDA1Nb3VudGFpbiBWaWV3MRMwEQYDVQQKDApHb29nbGUgTExDMRUw +EwYDVQQDDAwqLmdvb2dsZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAASs +8tMhHKTNkKBHuyC9u0qbTibi9ZkpyvkFSPhBziOsLn7uDkU/PSKjHnSCswip07o9 +F0kYWilWXKKxB5w2QQ0qo4IFHDCCBRgwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDgYD +VR0PAQH/BAQDAgeAMIID4QYDVR0RBIID2DCCA9SCDCouZ29vZ2xlLmNvbYINKi5h +bmRyb2lkLmNvbYIWKi5hcHBlbmdpbmUuZ29vZ2xlLmNvbYISKi5jbG91ZC5nb29n +bGUuY29tghQqLmRiODMzOTUzLmdvb2dsZS5jboIGKi5nLmNvgg4qLmdjcC5ndnQy +LmNvbYIWKi5nb29nbGUtYW5hbHl0aWNzLmNvbYILKi5nb29nbGUuY2GCCyouZ29v +Z2xlLmNsgg4qLmdvb2dsZS5jby5pboIOKi5nb29nbGUuY28uanCCDiouZ29vZ2xl +LmNvLnVrgg8qLmdvb2dsZS5jb20uYXKCDyouZ29vZ2xlLmNvbS5hdYIPKi5nb29n +bGUuY29tLmJygg8qLmdvb2dsZS5jb20uY2+CDyouZ29vZ2xlLmNvbS5teIIPKi5n +b29nbGUuY29tLnRygg8qLmdvb2dsZS5jb20udm6CCyouZ29vZ2xlLmRlggsqLmdv +b2dsZS5lc4ILKi5nb29nbGUuZnKCCyouZ29vZ2xlLmh1ggsqLmdvb2dsZS5pdIIL +Ki5nb29nbGUubmyCCyouZ29vZ2xlLnBsggsqLmdvb2dsZS5wdIISKi5nb29nbGVh +ZGFwaXMuY29tgg8qLmdvb2dsZWFwaXMuY26CFCouZ29vZ2xlY29tbWVyY2UuY29t +ghEqLmdvb2dsZXZpZGVvLmNvbYIMKi5nc3RhdGljLmNugg0qLmdzdGF0aWMuY29t +ggoqLmd2dDEuY29tggoqLmd2dDIuY29tghQqLm1ldHJpYy5nc3RhdGljLmNvbYIM +Ki51cmNoaW4uY29tghAqLnVybC5nb29nbGUuY29tghYqLnlvdXR1YmUtbm9jb29r +aWUuY29tgg0qLnlvdXR1YmUuY29tghYqLnlvdXR1YmVlZHVjYXRpb24uY29tggcq +Lnl0LmJlggsqLnl0aW1nLmNvbYIaYW5kcm9pZC5jbGllbnRzLmdvb2dsZS5jb22C +C2FuZHJvaWQuY29tghtkZXZlbG9wZXIuYW5kcm9pZC5nb29nbGUuY26CHGRldmVs +b3BlcnMuYW5kcm9pZC5nb29nbGUuY26CBGcuY2+CBmdvby5nbIIUZ29vZ2xlLWFu +YWx5dGljcy5jb22CCmdvb2dsZS5jb22CEmdvb2dsZWNvbW1lcmNlLmNvbYIYc291 +cmNlLmFuZHJvaWQuZ29vZ2xlLmNuggp1cmNoaW4uY29tggp3d3cuZ29vLmdsggh5 +b3V0dS5iZYILeW91dHViZS5jb22CFHlvdXR1YmVlZHVjYXRpb24uY29tggV5dC5i +ZTBoBggrBgEFBQcBAQRcMFowLQYIKwYBBQUHMAKGIWh0dHA6Ly9wa2kuZ29vZy9n +c3IyL0dUU0dJQUczLmNydDApBggrBgEFBQcwAYYdaHR0cDovL29jc3AucGtpLmdv +b2cvR1RTR0lBRzMwHQYDVR0OBBYEFK/WqypxoW4KZ4D8CDU5lyVLJXPNMAwGA1Ud +EwEB/wQCMAAwHwYDVR0jBBgwFoAUd8K4UJpndnaxLcKG0IOgfqZ+ukswIQYDVR0g +BBowGDAMBgorBgEEAdZ5AgUDMAgGBmeBDAECAjAxBgNVHR8EKjAoMCagJKAihiBo +dHRwOi8vY3JsLnBraS5nb29nL0dUU0dJQUczLmNybDANBgkqhkiG9w0BAQsFAAOC +AQEAbi8VuaNKx/otlEsrZ8+A0VbNvjOaQqqYodBbcu+/0MjGPLn4H9TKGVjsFtbY +piod3iX72Pg7X1WoQIoJUcybmZk64jocUBZOdZkZe2bjTAf6JQg9v7jh1pXgsEvv +UJ/86PBm6HsWAM2oMcIEOYO1e0/X0wJc1TogJn5/jTMA6u6JF4aQCLe1izgCSTeY +1efJiOYjVLfh/24+72yNpbS1z7whRVEHreXe2j2CrSiXnk60Wp7SZ88Ws1G7YPqa +Xqs1gJBb41sPz2dnR1vVIurciU6AD5nROQhhVWRF789Qf92gotfvvQDGrIcX2igm +j+CcQEW13qYWL+H8gReGc+vsvg== +-----END CERTIFICATE----- diff --git a/tests/integration/states/test_x509.py b/tests/integration/states/test_x509.py index a21339863f9d..763d806ee4d9 100644 --- a/tests/integration/states/test_x509.py +++ b/tests/integration/states/test_x509.py @@ -2,13 +2,16 @@ from __future__ import absolute_import, unicode_literals import os import logging + +import salt.utils.files +from salt.ext import six + +from tests.support.helpers import with_tempfile from tests.support.paths import BASE_FILES from tests.support.case import ModuleCase from tests.support.unit import skipIf from tests.support.mixins import SaltReturnAssertsMixin -import salt.utils.files - try: import M2Crypto # pylint: disable=W0611 HAS_M2CRYPTO = True @@ -19,48 +22,42 @@ log = logging.getLogger(__name__) +@skipIf(not HAS_M2CRYPTO, 'Skip when no M2Crypto found') class x509Test(ModuleCase, SaltReturnAssertsMixin): - def tearDown(self): - paths = [ - '/test-49027.crt', - '/test-ca-49008.key', - '/test-ca-49008.crt', - ] - for path in paths: - try: - os.remove(path) - except Exception: - pass - - @staticmethod - def get_cert_lines(path): - lines = [] - started = False - with salt.utils.files.fopen(path, 'rb') as fp: - for line in fp: - if line.find(b'-----BEGIN CERTIFICATE-----') != -1: - started = True - continue - if line.find(b'-----END CERTIFICATE-----') != -1: - break - if started: - lines.append(line.strip()) - return lines - - @skipIf(not HAS_M2CRYPTO, 'Skip when no M2Crypto found') - def test_issue_49027(self): - expected = self.get_cert_lines(os.path.join(BASE_FILES, 'issue-49027.sls')) - started = False - ret = self.run_function('state.sls', ['issue-49027']) - log.warn("ret = %s", repr(ret)) - self.assertSaltTrueReturn(ret) - self.assertEqual(expected, self.get_cert_lines('/test-49027.crt')) - - @skipIf(not HAS_M2CRYPTO, 'Skip when no M2Crypto found') - def test_issue_49008(self): - ret = self.run_function('state.sls', ['issue-49008']) - log.warn("ret = %s", repr(ret)) - self.assertSaltTrueReturn(ret) - self.assertTrue(os.path.exists('/test-ca-49008.key')) - self.assertTrue(os.path.exists('/test-ca-49008.crt')) + @classmethod + def setUpClass(cls): + cert_path = os.path.join(BASE_FILES, 'x509_test.crt') + with salt.utils.files.fopen(cert_path) as fp: + cls.x509_cert_text = fp.read() + + def run_function(self, *args, **kwargs): + ret = super(x509Test, self).run_function(*args, **kwargs) + log.debug('ret = %s', ret) + return ret + + @with_tempfile(suffix='.pem', create=False) + def test_issue_49027(self, pemfile): + ret = self.run_state( + 'x509.pem_managed', + name=pemfile, + text=self.x509_cert_text) + assert isinstance(ret, dict), ret + ret = ret[next(iter(ret))] + assert ret.get('result') is True, ret + with salt.utils.files.fopen(pemfile) as fp: + result = fp.readlines() + self.assertEqual(self.x509_cert_text.splitlines(True), result) + + @with_tempfile(suffix='.crt', create=False) + @with_tempfile(suffix='.key', create=False) + def test_issue_49008(self, keyfile, crtfile): + ret = self.run_function( + 'state.apply', + ['issue-49008'], + pillar={'keyfile': keyfile, 'crtfile': crtfile}) + assert isinstance(ret, dict), ret + for state_result in six.itervalues(ret): + assert state_result['result'] is True, state_result + assert os.path.exists(keyfile) + assert os.path.exists(crtfile) From e9714126ac5d7e2f56e27953bff3cf81c3147ae9 Mon Sep 17 00:00:00 2001 From: Mathieu Parent Date: Wed, 5 Dec 2018 07:14:09 +0100 Subject: [PATCH 07/55] git_pillar: Add support for all_saltenvs parameter On the road to #32245. --- salt/pillar/git_pillar.py | 23 ++- salt/utils/gitfs.py | 11 +- tests/integration/pillar/test_git_pillar.py | 188 ++++++++++++++++++++ 3 files changed, 220 insertions(+), 2 deletions(-) diff --git a/salt/pillar/git_pillar.py b/salt/pillar/git_pillar.py index aecd8cb6682a..07f13b4ee98f 100644 --- a/salt/pillar/git_pillar.py +++ b/salt/pillar/git_pillar.py @@ -329,6 +329,27 @@ file in the same pillar environment. - Salt versions prior to 2018.3.4 ignore the ``root`` parameter when ``mountpoint`` is set. + +.. _git-pillar-all_saltenvs: + +all_saltenvs +~~~~~~~~~~~~ + +.. versionadded:: 2018.3.4 + +When ``__env__`` is specified as the branch name, ``all_saltenvs`` per-remote configuration parameter overrides the logic Salt uses to map branches/tags to pillar environments (i.e. pillarenvs). This allows a single branch/tag to appear in all saltenvs. Example: + +.. code-block:: yaml + + ext_pillar: + - git: + - __env__ https://mydomain.tld/top.git + - all_saltenvs: master + - __env__ https://mydomain.tld/pillar-nginx.git: + - mountpoint: web/server/ + - __env__ https://mydomain.tld/pillar-appdata.git: + - mountpoint: web/server/ + ''' from __future__ import absolute_import, print_function, unicode_literals @@ -348,7 +369,7 @@ from salt.ext import six PER_REMOTE_OVERRIDES = ('env', 'root', 'ssl_verify', 'refspecs') -PER_REMOTE_ONLY = ('name', 'mountpoint') +PER_REMOTE_ONLY = ('name', 'mountpoint', 'all_saltenvs') GLOBAL_ONLY = ('base', 'branch') # Set up logging diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 7de529014ca6..1e2383745b7c 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -984,6 +984,11 @@ def get_checkout_target(self): Resolve dynamically-set branch ''' if self.role == 'git_pillar' and self.branch == '__env__': + try: + return self.all_saltenvs + except AttributeError: + # all_saltenvs not configured for this remote + pass target = self.opts.get('pillarenv') \ or self.opts.get('saltenv') \ or 'base' @@ -2971,7 +2976,11 @@ def checkout(self): cachedir = self.do_checkout(repo) if cachedir is not None: # Figure out which environment this remote should be assigned - if repo.env: + if repo.branch == '__env__' and hasattr(repo, 'all_saltenvs'): + env = self.opts.get('pillarenv') \ + or self.opts.get('saltenv') \ + or self.opts.get('git_pillar_base') + elif repo.env: env = repo.env else: env = 'base' if repo.branch == repo.base else repo.get_checkout_target() diff --git a/tests/integration/pillar/test_git_pillar.py b/tests/integration/pillar/test_git_pillar.py index dc58f75c12bf..5d9a374f6e50 100644 --- a/tests/integration/pillar/test_git_pillar.py +++ b/tests/integration/pillar/test_git_pillar.py @@ -460,6 +460,34 @@ def test_root_and_mountpoint_parameters(self): ''') self.assertEqual(ret, expected) + def test_all_saltenvs(self): + ''' + Test all_saltenvs parameter. + ''' + ret = self.get_pillar('''\ + file_ignore_regex: [] + file_ignore_glob: [] + git_pillar_provider: gitpython + cachedir: {cachedir} + extension_modules: {extmods} + pillarenv: dev + ext_pillar: + - git: + - __env__ {url_extra_repo}: + - all_saltenvs: master + - __env__ {url}: + - mountpoint: nowhere + ''') + self.assertEqual( + ret, + {'branch': 'dev', + 'motd': 'The force will be with you. Always.', + 'mylist': ['dev'], + 'mydict': {'dev': True, + 'nested_list': ['dev'], + 'nested_dict': {'dev': True}}} + ) + @destructiveTest @skipIf(NO_MOCK, NO_MOCK_REASON) @@ -1613,6 +1641,107 @@ def test_root_and_mountpoint_parameters(self, grains): ''') self.assertEqual(ret, expected) + @requires_system_grains + def test_all_saltenvs(self, grains): + ''' + Test all_saltenvs parameter. + ''' + expected = {'branch': 'dev', + 'motd': 'The force will be with you. Always.', + 'mylist': ['dev'], + 'mydict': {'dev': True, + 'nested_list': ['dev'], + 'nested_dict': {'dev': True} + } + } + + # Test with passphraseless key and global credential options + ret = self.get_pillar('''\ + file_ignore_regex: [] + file_ignore_glob: [] + git_pillar_provider: pygit2 + git_pillar_pubkey: {pubkey_nopass} + git_pillar_privkey: {privkey_nopass} + cachedir: {cachedir} + extension_modules: {extmods} + pillarenv: dev + ext_pillar: + - git: + - __env__ {url_extra_repo}: + - all_saltenvs: master + - __env__ {url}: + - mountpoint: nowhere + ''') + self.assertEqual(ret, expected) + + # Test with passphraseless key and per-repo credential options + ret = self.get_pillar('''\ + file_ignore_regex: [] + file_ignore_glob: [] + git_pillar_provider: pygit2 + cachedir: {cachedir} + extension_modules: {extmods} + pillarenv: dev + ext_pillar: + - git: + - __env__ {url_extra_repo}: + - all_saltenvs: master + - pubkey: {pubkey_nopass} + - privkey: {privkey_nopass} + - __env__ {url}: + - mountpoint: nowhere + - pubkey: {pubkey_nopass} + - privkey: {privkey_nopass} + ''') + self.assertEqual(ret, expected) + + if grains['os_family'] == 'Debian': + # passphrase-protected currently does not work here + return + + # Test with passphrase-protected key and global credential options + ret = self.get_pillar('''\ + file_ignore_regex: [] + file_ignore_glob: [] + git_pillar_provider: pygit2 + git_pillar_pubkey: {pubkey_withpass} + git_pillar_privkey: {privkey_withpass} + git_pillar_passphrase: {passphrase} + cachedir: {cachedir} + extension_modules: {extmods} + pillarenv: dev + ext_pillar: + - git: + - __env__ {url_extra_repo}: + - all_saltenvs: master + - __env__ {url}: + - mountpoint: nowhere + ''') + self.assertEqual(ret, expected) + + # Test with passphrase-protected key and per-repo credential options + ret = self.get_pillar('''\ + file_ignore_regex: [] + file_ignore_glob: [] + git_pillar_provider: pygit2 + cachedir: {cachedir} + extension_modules: {extmods} + pillarenv: dev + ext_pillar: + - git: + - __env__ {url_extra_repo}: + - all_saltenvs: master + - pubkey: {pubkey_nopass} + - privkey: {privkey_nopass} + - passphrase: {passphrase} + - __env__ {url}: + - mountpoint: nowhere + - pubkey: {pubkey_nopass} + - privkey: {privkey_nopass} + - passphrase: {passphrase} + ''') + self.assertEqual(ret, expected) + @skipIf(NO_MOCK, NO_MOCK_REASON) @skipIf(_windows_or_mac(), 'minion is windows or mac') @@ -1962,6 +2091,34 @@ def test_root_and_mountpoint_parameters(self): ''') self.assertEqual(ret, expected) + def test_all_saltenvs(self): + ''' + Test all_saltenvs parameter. + ''' + ret = self.get_pillar('''\ + file_ignore_regex: [] + file_ignore_glob: [] + git_pillar_provider: pygit2 + cachedir: {cachedir} + extension_modules: {extmods} + pillarenv: dev + ext_pillar: + - git: + - __env__ {url_extra_repo}: + - all_saltenvs: master + - __env__ {url}: + - mountpoint: nowhere + ''') + self.assertEqual( + ret, + {'branch': 'dev', + 'motd': 'The force will be with you. Always.', + 'mylist': ['dev'], + 'mydict': {'dev': True, + 'nested_list': ['dev'], + 'nested_dict': {'dev': True}}} + ) + @skipIf(NO_MOCK, NO_MOCK_REASON) @skipIf(_windows_or_mac(), 'minion is windows or mac') @@ -2531,3 +2688,34 @@ def test_root_and_mountpoint_parameters(self): - env: base ''') self.assertEqual(ret, expected) + + def test_all_saltenvs(self): + ''' + Test all_saltenvs parameter. + ''' + ret = self.get_pillar('''\ + file_ignore_regex: [] + file_ignore_glob: [] + git_pillar_provider: pygit2 + git_pillar_user: {user} + git_pillar_password: {password} + git_pillar_insecure_auth: True + cachedir: {cachedir} + extension_modules: {extmods} + pillarenv: dev + ext_pillar: + - git: + - __env__ {url_extra_repo}: + - all_saltenvs: master + - __env__ {url}: + - mountpoint: nowhere + ''') + self.assertEqual( + ret, + {'branch': 'dev', + 'motd': 'The force will be with you. Always.', + 'mylist': ['dev'], + 'mydict': {'dev': True, + 'nested_list': ['dev'], + 'nested_dict': {'dev': True}}} + ) From ef160a67d66f3eac78f62e982976584b330eabd6 Mon Sep 17 00:00:00 2001 From: Mathieu Parent Date: Fri, 7 Dec 2018 06:37:16 +0100 Subject: [PATCH 08/55] gitfs/git_pillar: Fix UnicodeDecodeError while cleaning stale refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 2018-12-06 11:57:11,495 [salt.utils.gitfs :2341][ERROR ][866] Exception caught while fetching gitfs remote '__env__ https://git.example.org/somerepo.git': 'ascii' codec can't decode byte 0xc3 in position 0: ordinal not in range(128) Traceback (most recent call last): File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 2329, in fetch_remotes if repo.fetch(): File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 757, in fetch return self._fetch() File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 1271, in _fetch cleaned = self.clean_stale_refs() File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 605, in clean_stale_refs if line.startswith(marker): UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 0: ordinal not in range(128) The environement variables are not sanitized, and as we use LANG=fr_FR.utf8. Calling the same command prints non-ascii: root@salt-master:/var/cache/salt/master/git_pillar/51958f355141ebce331e186a5230c20f2d0264025054eeb2fc2c5b3b81ab53a0# git remote prune origin Élimination de origin URL : https://git.example.org/somerepo.git * [éliminé] origin/toto Inspired by https://github.com/gitpython-developers/GitPython/blob/a8591a094a768d73e6efb5a698f74d354c989291/git/cmd.py#L694 --- salt/utils/gitfs.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 7de529014ca6..41bae0f747b4 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -584,10 +584,20 @@ def clean_stale_refs(self): ''' cleaned = [] cmd_str = 'git remote prune origin' + + # Attempt to force all output to plain ascii english, which is what some parsing code + # may expect. + # According to stackoverflow (http://goo.gl/l74GC8), we are setting LANGUAGE as well + # just to be sure. + env = os.environ.copy() + env["LANGUAGE"] = "C" + env["LC_ALL"] = "C" + cmd = subprocess.Popen( shlex.split(cmd_str), close_fds=not salt.utils.platform.is_windows(), cwd=os.path.dirname(self.gitdir), + env=env, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) output = cmd.communicate()[0] From 87a04df512c376b86bf3824bf91d90dd6a97ea98 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sat, 8 Dec 2018 00:12:58 +0000 Subject: [PATCH 09/55] Fix #48801 min, max, inact, and warn changes The _changes method was not properlt reporting mindays, maxdays, inactdays and warndays values --- salt/states/user.py | 8 +++---- tests/unit/states/test_user.py | 41 ++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/salt/states/user.py b/salt/states/user.py index dfcddc27c037..c23dcc211765 100644 --- a/salt/states/user.py +++ b/salt/states/user.py @@ -141,13 +141,13 @@ def _changes(name, change['empty_password'] = True if date is not None and lshad['lstchg'] != date: change['date'] = date - if mindays and mindays is not 0 and lshad['min'] != mindays: + if mindays is not None and lshad['min'] != mindays: change['mindays'] = mindays - if maxdays and maxdays is not 999999 and lshad['max'] != maxdays: + if maxdays is not None and lshad['max'] != maxdays: change['maxdays'] = maxdays - if inactdays and inactdays is not 0 and lshad['inact'] != inactdays: + if inactdays is not None and lshad['inact'] != inactdays: change['inactdays'] = inactdays - if warndays and warndays is not 7 and lshad['warn'] != warndays: + if warndays is not None and lshad['warn'] != warndays: change['warndays'] = warndays if expire and lshad['expire'] != expire: change['expire'] = expire diff --git a/tests/unit/states/test_user.py b/tests/unit/states/test_user.py index 9a6effe0abf6..63fd26e2c299 100644 --- a/tests/unit/states/test_user.py +++ b/tests/unit/states/test_user.py @@ -5,6 +5,8 @@ # Import Python Libs from __future__ import absolute_import, print_function, unicode_literals +import os +import logging # Import Salt Testing Libs from tests.support.mixins import LoaderModuleMockMixin @@ -20,6 +22,7 @@ # Import Salt Libs import salt.states.user as user +log = logging.getLogger(__name__) @skipIf(NO_MOCK, NO_MOCK_REASON) class UserTestCase(TestCase, LoaderModuleMockMixin): @@ -209,3 +212,41 @@ def test_absent(self): ret.update({'comment': 'User salt is not present', 'result': True}) self.assertDictEqual(user.absent('salt'), ret) + + def test_changes(self): + ''' + Test salt.states.user._changes + ''' + mock_info = MagicMock(return_value= + {'uid': 5000, + 'gid': 5000, + 'groups': ['foo'], + 'home': '/home/foo', + 'fullname': 'Foo Bar'} + ) + shadow_info = MagicMock(return_value= + {'min': 2 , + 'max': 88888, + 'inact': 77, + 'warn': 14, + } + ) + shadow_hash = MagicMock(return_value='abcd') + dunder_salt = {'user.info': mock_info, + 'shadow.info': shadow_info, + 'shadow.default_hash': shadow_hash, + 'file.group_to_gid': MagicMock(side_effect=['foo']), + 'file.gid_to_group': MagicMock(side_effect=[5000])} + # side_effect used because these mocks should only be called once + with patch.dict(user.__grains__, {'kernel': 'Linux'}), \ + patch.dict(user.__salt__, dunder_salt), \ + patch.dict(user.__opts__, {"test": False}), \ + patch('os.path.isdir', lambda x : True): + ret = user._changes('foo', maxdays=999999, inactdays=0, warndays=7) + assert ret == { + 'maxdays': 999999, + 'mindays': 0, + 'fullname': '', + 'warndays': 7, + 'inactdays': 0 + } From 6b9522343e382b8b8d3e3cee4540e5a9d5738ddd Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 7 Dec 2018 17:32:48 -0700 Subject: [PATCH 10/55] fix linter errors --- tests/unit/states/test_user.py | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/tests/unit/states/test_user.py b/tests/unit/states/test_user.py index 63fd26e2c299..52f79970457e 100644 --- a/tests/unit/states/test_user.py +++ b/tests/unit/states/test_user.py @@ -5,7 +5,6 @@ # Import Python Libs from __future__ import absolute_import, print_function, unicode_literals -import os import logging # Import Salt Testing Libs @@ -24,6 +23,7 @@ log = logging.getLogger(__name__) + @skipIf(NO_MOCK, NO_MOCK_REASON) class UserTestCase(TestCase, LoaderModuleMockMixin): ''' @@ -217,18 +217,21 @@ def test_changes(self): ''' Test salt.states.user._changes ''' - mock_info = MagicMock(return_value= - {'uid': 5000, - 'gid': 5000, - 'groups': ['foo'], - 'home': '/home/foo', - 'fullname': 'Foo Bar'} + mock_info = MagicMock( + return_value={ + 'uid': 5000, + 'gid': 5000, + 'groups': ['foo'], + 'home': '/home/foo', + 'fullname': 'Foo Bar', + } ) - shadow_info = MagicMock(return_value= - {'min': 2 , - 'max': 88888, - 'inact': 77, - 'warn': 14, + shadow_info = MagicMock( + return_value={ + 'min': 2, + 'max': 88888, + 'inact': 77, + 'warn': 14, } ) shadow_hash = MagicMock(return_value='abcd') @@ -237,11 +240,14 @@ def test_changes(self): 'shadow.default_hash': shadow_hash, 'file.group_to_gid': MagicMock(side_effect=['foo']), 'file.gid_to_group': MagicMock(side_effect=[5000])} + def mock_exists(*args): + return True + # side_effect used because these mocks should only be called once with patch.dict(user.__grains__, {'kernel': 'Linux'}), \ patch.dict(user.__salt__, dunder_salt), \ patch.dict(user.__opts__, {"test": False}), \ - patch('os.path.isdir', lambda x : True): + patch('os.path.isdir', mock_exists): ret = user._changes('foo', maxdays=999999, inactdays=0, warndays=7) assert ret == { 'maxdays': 999999, From b905cf6c124f3844056616f2c98ca253a9f06773 Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Tue, 28 Aug 2018 01:09:51 -0500 Subject: [PATCH 11/55] Added support for dns_{nameservers,search} being a list or string. Conflicts: - salt/templates/debian_ip/debian_eth.jinja --- salt/templates/debian_ip/debian_eth.jinja | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/salt/templates/debian_ip/debian_eth.jinja b/salt/templates/debian_ip/debian_eth.jinja index 09fb34e4c8fe..331ea651aee5 100644 --- a/salt/templates/debian_ip/debian_eth.jinja +++ b/salt/templates/debian_ip/debian_eth.jinja @@ -30,8 +30,12 @@ {%endif%}{% if interface.unit %} unit {{interface.unit}} {%endif%}{% if interface.options %} options {{interface.options}} {%endif%}{% if interface.master %} bond-master {{interface.master}} -{%endif%}{% if interface.dns_nameservers %} dns-nameservers {%for item in interface.dns_nameservers %}{{item}} {%endfor%} -{%endif%}{% if interface.dns_search %} dns-search {% for item in interface.dns_search %}{{item}} {%endfor%} +{%endif%}{% if interface.dns_nameservers %} dns-nameservers {% + if interface.dns_nameservers is string %}{{ interface.dns_nameservers }}{% + else %}{{ interface.dns_nameservers|join(" ") }}{% endif %} +{%endif%}{% if interface.dns_search %} dns-search {% + if interface.dns_search is string %}{{interface.dns_search }}{% + else %}{{ interface.dns_search|join(" ") }}{% endif %} {%endif%}{% if interface.ethtool %}{%for item in interface.ethtool_keys %} {{item}} {{interface.ethtool[item]}} {%endfor%}{%endif%}{% if interface.bonding %}{%for item in interface.bonding_keys %} bond-{{item}} {{interface.bonding[item]}} {%endfor%}{%endif%}{% if interface.bridging %}{%for item in interface.bridging_keys %} bridge_{{item}} {{interface.bridging[item]}} @@ -94,8 +98,12 @@ {%endif%}{% if interface.unit %} unit {{interface.unit}} {%endif%}{% if interface.options %} options {{interface.options}} {%endif%}{% if interface.master %} bond-master {{interface.master}} -{%endif%}{% if interface.dns_nameservers %} dns-nameservers {%for item in interface.dns_nameservers %}{{item}} {%endfor%} -{%endif%}{% if interface.dns_search %} dns-search {% for item in interface.dns_search %}{{item}} {%endfor%} +{%endif%}{% if interface.dns_nameservers %} dns-nameservers {% + if interface.dns_nameservers is string %}{{ interface.dns_nameservers }}{% + else %}{{ interface.dns_nameservers|join(" ") }}{% endif %} +{%endif%}{% if interface.dns_search %} dns-search {% + if interface.dns_search is string %}{{interface.dns_search }}{% + else %}{{ interface.dns_search|join(" ") }}{% endif %} {%endif%}{% if interface.ethtool %}{%for item in interface.ethtool_keys %} {{item}} {{interface.ethtool[item]}} {%endfor%}{%endif%}{% if interface.bonding %}{%for item in interface.bonding_keys %} bond-{{item}} {{interface.bonding[item]}} {%endfor%}{%endif%}{% if interface.bridging %}{%for item in interface.bridging_keys %} bridge_{{item}} {{interface.bridging[item]}} From cf61c8d9cf97b2eede93658add0c504830e2e2c6 Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Tue, 28 Aug 2018 01:10:27 -0500 Subject: [PATCH 12/55] Added a bunch of unit tests for modules.debian_ip.build_interface(). --- tests/unit/modules/test_debian_ip.py | 278 +++++++++++++++++++++++++++ 1 file changed, 278 insertions(+) diff --git a/tests/unit/modules/test_debian_ip.py b/tests/unit/modules/test_debian_ip.py index 448ae44a69c3..4d65c3361c6b 100644 --- a/tests/unit/modules/test_debian_ip.py +++ b/tests/unit/modules/test_debian_ip.py @@ -174,6 +174,284 @@ def test_get_interface(self): with patch.object(jinja2.Environment, 'get_template', mock): self.assertEqual(debian_ip.get_interface('lo'), '') + # 'build_interface' function tests: 12 + + def test_build_interface(self): + ''' + Test if salt correctly builds interfaces. + ''' + interfaces = [ + # IPv4-only interface; single address + {'iface_name': 'eth1', 'iface_type': 'eth', 'enabled': True, + 'settings': { + 'proto': 'static', + 'ipaddr': '192.168.4.9', + 'netmask': '255.255.255.0', + 'gateway': '192.168.4.1', + 'enable_ipv6': False, + 'noifupdown': True, + }, + 'return': [ + 'auto eth1\n', + 'iface eth1 inet static\n', + ' address 192.168.4.9\n', + ' netmask 255.255.255.0\n', + ' gateway 192.168.4.1\n', + '\n']}, + # IPv6-only; single address + {'iface_name': 'eth2', 'iface_type': 'eth', 'enabled': True, + 'settings': { + 'proto': 'static', + 'ipv6proto': 'static', + 'ipv6ipaddr': '2001:db8:dead:beef::3', + 'ipv6netmask': '64', + 'ipv6gateway': '2001:db8:dead:beef::1', + 'enable_ipv6': True, + 'noifupdown': True, + }, + 'return': [ + 'auto eth2\n', + 'iface eth2 inet6 static\n', + ' address 2001:db8:dead:beef::3\n', + ' netmask 64\n', + ' gateway 2001:db8:dead:beef::1\n', + '\n']}, + # IPv4 and IPv6; shared/overridden settings + {'iface_name': 'eth3', 'iface_type': 'eth', 'enabled': True, + 'settings': { + 'proto': 'static', + 'ipaddr': '192.168.4.9', + 'netmask': '255.255.255.0', + 'gateway': '192.168.4.1', + 'ipv6proto': 'static', + 'ipv6ipaddr': '2001:db8:dead:beef::3', + 'ipv6netmask': '64', + 'ipv6gateway': '2001:db8:dead:beef::1', + 'ttl': '18', # shared + 'ipv6ttl': '15', # overriden for v6 + 'mtu': '1480', # shared + 'enable_ipv6': True, + 'noifupdown': True, + }, + 'return': [ + 'auto eth3\n', + 'iface eth3 inet static\n', + ' address 192.168.4.9\n', + ' netmask 255.255.255.0\n', + ' gateway 192.168.4.1\n', + ' ttl 18\n', + ' mtu 1480\n', + 'iface eth3 inet6 static\n', + ' address 2001:db8:dead:beef::3\n', + ' netmask 64\n', + ' gateway 2001:db8:dead:beef::1\n', + ' ttl 15\n', + ' mtu 1480\n', + '\n']}, + # Slave iface + {'iface_name': 'eth4', 'iface_type': 'slave', 'enabled': True, + 'settings': { + 'master': 'bond0', + 'noifupdown': True, + }, + 'return': [ + 'auto eth4\n', + 'iface eth4 inet manual\n', + ' bond-master bond0\n', + '\n']}, + # Bond; with address IPv4 and IPv6 address; slaves as string + {'iface_name': 'bond5', 'iface_type': 'bond', 'enabled': True, + 'settings': { + 'proto': 'static', + 'ipaddr': '10.1.0.14', + 'netmask': '255.255.255.0', + 'gateway': '10.1.0.1', + 'ipv6proto': 'static', + 'ipv6ipaddr': '2001:db8:dead:c0::3', + 'ipv6netmask': '64', + 'ipv6gateway': '2001:db8:dead:c0::1', + 'mode': '802.3ad', + 'slaves': 'eth4 eth5', + 'enable_ipv6': True, + 'noifupdown': True, + }, + 'return': [ + 'auto bond5\n', + 'iface bond5 inet static\n', + ' address 10.1.0.14\n', + ' netmask 255.255.255.0\n', + ' gateway 10.1.0.1\n', + ' bond-ad_select 0\n', + ' bond-downdelay 200\n', + ' bond-lacp_rate 0\n', + ' bond-miimon 100\n', + ' bond-mode 4\n', + ' bond-slaves eth4 eth5\n', + ' bond-updelay 0\n', + ' bond-use_carrier on\n', + 'iface bond5 inet6 static\n', + ' address 2001:db8:dead:c0::3\n', + ' netmask 64\n', + ' gateway 2001:db8:dead:c0::1\n', + # TODO: I suspect there should be more here. + '\n']}, + # Bond; with address IPv4 and IPv6 address; slaves as list + {'iface_name': 'bond6', 'iface_type': 'bond', 'enabled': True, + 'settings': { + 'proto': 'static', + 'ipaddr': '10.1.0.14', + 'netmask': '255.255.255.0', + 'gateway': '10.1.0.1', + 'ipv6proto': 'static', + 'ipv6ipaddr': '2001:db8:dead:c0::3', + 'ipv6netmask': '64', + 'ipv6gateway': '2001:db8:dead:c0::1', + 'mode': '802.3ad', + # TODO: Need to add this support + #'slaves': ['eth4', 'eth5'], + 'slaves': 'eth4 eth5', + 'enable_ipv6': True, + 'noifupdown': True, + }, + 'return': [ + 'auto bond6\n', + 'iface bond6 inet static\n', + ' address 10.1.0.14\n', + ' netmask 255.255.255.0\n', + ' gateway 10.1.0.1\n', + ' bond-ad_select 0\n', + ' bond-downdelay 200\n', + ' bond-lacp_rate 0\n', + ' bond-miimon 100\n', + ' bond-mode 4\n', + ' bond-slaves eth4 eth5\n', + ' bond-updelay 0\n', + ' bond-use_carrier on\n', + 'iface bond6 inet6 static\n', + ' address 2001:db8:dead:c0::3\n', + ' netmask 64\n', + ' gateway 2001:db8:dead:c0::1\n', + # TODO: I suspect there should be more here. + '\n']}, + # Bond VLAN; with IPv4 address + {'iface_name': 'bond1.7', 'iface_type': 'vlan', 'enabled': True, + 'settings': { + 'proto': 'static', + 'ipaddr': '10.7.0.8', + 'netmask': '255.255.255.0', + 'gateway': '10.7.0.1', + 'slaves': 'eth6 eth7', + 'mode': '802.3ad', + 'enable_ipv6': False, + 'noifupdown': True, + }, + 'return': [ + 'auto bond1.7\n', + 'iface bond1.7 inet static\n', + ' vlan-raw-device bond1\n', + ' address 10.7.0.8\n', + ' netmask 255.255.255.0\n', + ' gateway 10.7.0.1\n', + ' mode 802.3ad\n', + '\n']}, + # Bond; without address + {'iface_name': 'bond1.8', 'iface_type': 'vlan', 'enabled': True, + 'settings': { + 'proto': 'static', + 'slaves': 'eth6 eth7', + 'mode': '802.3ad', + 'enable_ipv6': False, + 'noifupdown': True, + }, + 'return': [ + 'auto bond1.8\n', + 'iface bond1.8 inet static\n', + ' vlan-raw-device bond1\n', + ' mode 802.3ad\n', + '\n']}, + # DNS NS as list + {'iface_name': 'eth9', 'iface_type': 'eth', 'enabled': True, + 'settings': { + 'proto': 'static', + 'ipaddr': '192.168.4.9', + 'netmask': '255.255.255.0', + 'gateway': '192.168.4.1', + 'enable_ipv6': False, + 'noifupdown': True, + 'dns': ['8.8.8.8', '8.8.4.4'], + }, + 'return': [ + 'auto eth9\n', + 'iface eth9 inet static\n', + ' address 192.168.4.9\n', + ' netmask 255.255.255.0\n', + ' gateway 192.168.4.1\n', + ' dns-nameservers 8.8.8.8 8.8.4.4\n', + '\n']}, + # DNS NS as string + {'iface_name': 'eth10', 'iface_type': 'eth', 'enabled': True, + 'settings': { + 'proto': 'static', + 'ipaddr': '192.168.4.9', + 'netmask': '255.255.255.0', + 'gateway': '192.168.4.1', + 'enable_ipv6': False, + 'noifupdown': True, + 'dns': '8.8.8.8 8.8.4.4', + }, + 'return': [ + 'auto eth10\n', + 'iface eth10 inet static\n', + ' address 192.168.4.9\n', + ' netmask 255.255.255.0\n', + ' gateway 192.168.4.1\n', + ' dns-nameservers 8.8.8.8 8.8.4.4\n', + '\n']}, + # Loopback; with IPv4 and IPv6 address + {'iface_name': 'lo11', 'iface_type': 'loopback', 'enabled': True, + 'settings': { + 'proto': 'loopback', + 'ipaddr': '192.168.4.9', + 'netmask': '255.255.255.0', + 'gateway': '192.168.4.1', + 'ipv6ipaddr': 'fc00::1', + 'ipv6netmask': '128', + 'ipv6_autoconf': False, + 'enable_ipv6': True, + 'noifupdown': True, + }, + 'return': [ + 'auto lo11\n', + 'iface lo11 inet loopback\n', + ' address 192.168.4.9\n', + ' netmask 255.255.255.0\n', + ' gateway 192.168.4.1\n', + 'iface lo11 inet6 loopback\n', + ' address fc00::1\n', + ' netmask 128\n', + '\n']}, + # Loopback; without address + {'iface_name': 'lo12', 'iface_type': 'loopback', 'enabled': True, + 'settings': { + 'enable_ipv6': False, + 'noifupdown': True, + }, + 'return': [ + 'auto lo12\n', + 'iface lo12 inet loopback\n', + '\n']}, + ] + + for iface in interfaces: + # Skip tests that require __salt__['pkg.install']() + if iface['iface_type'] not in ['bridge', 'pppoe', 'vlan']: + self.assertListEqual(debian_ip.build_interface( + iface = iface['iface_name'], + iface_type = iface['iface_type'], + enabled = iface['enabled'], + **iface['settings']), + iface['return']) + # 'up' function tests: 1 def test_up(self): From 638f899e6d5269672cff4df59583222ef45f61f8 Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Tue, 28 Aug 2018 01:14:25 -0500 Subject: [PATCH 13/55] Added support for loopback devices to modules.debian_ip(). (Fixes: #38672) --- salt/states/network.py | 1 + tests/unit/modules/test_debian_ip.py | 59 +++++++++++++--------------- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/salt/states/network.py b/salt/states/network.py index 118d58b2ff83..f09cb418bd56 100644 --- a/salt/states/network.py +++ b/salt/states/network.py @@ -217,6 +217,7 @@ network.managed: - name: lo - type: eth + - proto: loopback - onboot: yes - userctl: no - ipv6_autoconf: no diff --git a/tests/unit/modules/test_debian_ip.py b/tests/unit/modules/test_debian_ip.py index 4d65c3361c6b..28a4d3249fee 100644 --- a/tests/unit/modules/test_debian_ip.py +++ b/tests/unit/modules/test_debian_ip.py @@ -76,34 +76,6 @@ def test_build_bond_data(self): with patch.dict(debian_ip.__grains__, {'osrelease': mock}): self.assertTrue(debian_ip.build_bond('bond0', test='True')) - # 'build_interface' function tests: 1 - - def test_build_interface(self): - ''' - Test if it builds an interface script for a network interface. - ''' - with patch('salt.modules.debian_ip._write_file_ifaces', - MagicMock(return_value='salt')): - self.assertEqual(debian_ip.build_interface('eth0', 'eth', 'enabled'), - ['s\n', 'a\n', 'l\n', 't\n']) - - self.assertTrue(debian_ip.build_interface('eth0', 'eth', 'enabled', - test='True')) - - with patch.object(debian_ip, '_parse_settings_eth', - MagicMock(return_value={'routes': []})): - self.assertRaises(AttributeError, debian_ip.build_interface, - 'eth0', 'bridge', 'enabled') - - self.assertRaises(AttributeError, debian_ip.build_interface, - 'eth0', 'slave', 'enabled') - - self.assertRaises(AttributeError, debian_ip.build_interface, - 'eth0', 'bond', 'enabled') - - self.assertTrue(debian_ip.build_interface('eth0', 'eth', 'enabled', - test='True')) - # 'build_routes' function tests: 2 def test_build_routes(self): @@ -174,12 +146,34 @@ def test_get_interface(self): with patch.object(jinja2.Environment, 'get_template', mock): self.assertEqual(debian_ip.get_interface('lo'), '') - # 'build_interface' function tests: 12 + # 'build_interface' function tests: 1 def test_build_interface(self): ''' - Test if salt correctly builds interfaces. + Test if it builds an interface script for a network interface. ''' + with patch('salt.modules.debian_ip._write_file_ifaces', + MagicMock(return_value='salt')): + self.assertEqual(debian_ip.build_interface('eth0', 'eth', 'enabled'), + ['s\n', 'a\n', 'l\n', 't\n']) + + self.assertTrue(debian_ip.build_interface('eth0', 'eth', 'enabled', + test='True')) + + with patch.object(debian_ip, '_parse_settings_eth', + MagicMock(return_value={'routes': []})): + self.assertRaises(AttributeError, debian_ip.build_interface, + 'eth0', 'bridge', 'enabled') + + self.assertRaises(AttributeError, debian_ip.build_interface, + 'eth0', 'slave', 'enabled') + + self.assertRaises(AttributeError, debian_ip.build_interface, + 'eth0', 'bond', 'enabled') + + self.assertTrue(debian_ip.build_interface('eth0', 'eth', 'enabled', + test='True')) + interfaces = [ # IPv4-only interface; single address {'iface_name': 'eth1', 'iface_type': 'eth', 'enabled': True, @@ -408,7 +402,7 @@ def test_build_interface(self): ' dns-nameservers 8.8.8.8 8.8.4.4\n', '\n']}, # Loopback; with IPv4 and IPv6 address - {'iface_name': 'lo11', 'iface_type': 'loopback', 'enabled': True, + {'iface_name': 'lo11', 'iface_type': 'eth', 'enabled': True, 'settings': { 'proto': 'loopback', 'ipaddr': '192.168.4.9', @@ -431,8 +425,9 @@ def test_build_interface(self): ' netmask 128\n', '\n']}, # Loopback; without address - {'iface_name': 'lo12', 'iface_type': 'loopback', 'enabled': True, + {'iface_name': 'lo12', 'iface_type': 'eth', 'enabled': True, 'settings': { + 'proto': 'loopback', 'enable_ipv6': False, 'noifupdown': True, }, From 601f434b068d279812607df00024cc442452b71f Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Tue, 28 Aug 2018 01:15:20 -0500 Subject: [PATCH 14/55] Added support for -ipaddrs and -ipv6ipaddrs to modules.debian_ip(). --- salt/modules/debian_ip.py | 5 +++++ salt/templates/debian_ip/debian_eth.jinja | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/salt/modules/debian_ip.py b/salt/modules/debian_ip.py index e4797e5023cf..2cee9e770bf1 100644 --- a/salt/modules/debian_ip.py +++ b/salt/modules/debian_ip.py @@ -1707,6 +1707,11 @@ def build_interface(iface, iface_type, enabled, **settings): if 'proto' not in settings: settings['proto'] = 'static' + if 'ipv6ipaddr' not in settings and 'ipv6ipaddrs' in settings: + settings['ipv6addr'] = settings['ipv6ipaddrs'].pop(0) + if 'ipaddr' not in settings and 'ipaddrs' in settings: + settings['ipaddr'] = settings['ipaddrs'].pop(0) + if iface_type == 'slave': settings['slave'] = 'yes' if 'master' not in settings: diff --git a/salt/templates/debian_ip/debian_eth.jinja b/salt/templates/debian_ip/debian_eth.jinja index 331ea651aee5..e44f4f36e96b 100644 --- a/salt/templates/debian_ip/debian_eth.jinja +++ b/salt/templates/debian_ip/debian_eth.jinja @@ -5,7 +5,8 @@ {% if interface.hwaddress %} hwaddress {{interface.hwaddress}} {%endif%}{% if interface.vlan_raw_device %} vlan-raw-device {{interface.vlan_raw_device}} {%endif%}{% if interface.address %} address {{interface.address}} -{%endif%}{% if interface.netmask %} netmask {{interface.netmask}} +{%endif%}{% if interface.addresses %}{%for addr in interface.addresses %} address {{addr}} +{%endfor%}{%endif%}{% if interface.netmask %} netmask {{interface.netmask}} {%endif%}{% if interface.broadcast %} broadcast {{interface.broadcast}} {%endif%}{% if interface.metric %} metric {{interface.metric}} {%endif%}{% if interface.gateway %} gateway {{interface.gateway}} @@ -73,7 +74,8 @@ {%endif%}{% if interface.dhcp %} dhcp {{interface.dhcp}}{# END V6ONLOPTS #} {%endif%}{% if interface.vlan_raw_device %} vlan-raw-device {{interface.vlan_raw_device}} {%endif%}{% if interface.address %} address {{interface.address}} -{%endif%}{% if interface.netmask %} netmask {{interface.netmask}} +{%endif%}{% if interface.addresses %}{% for addr in interface.addresses %} address {{addr}} +{%endfor%}{%endif%}{% if interface.netmask %} netmask {{interface.netmask}} {%endif%}{% if interface.broadcast %} broadcast {{interface.broadcast}} {%endif%}{% if interface.metric %} metric {{interface.metric}} {%endif%}{% if interface.gateway %} gateway {{interface.gateway}} From 4c7df5fd3c6aeab9941ed49923dc3870ce7a41b5 Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Tue, 28 Aug 2018 01:16:15 -0500 Subject: [PATCH 15/55] Cleaned up documentation/examples in states.network: - Updated debian-based distro comment - Fixed example of configuring a loopback device - Cleaned up copy/paste mistakes - Added more examples - Verified examples work on Debian 9 --- salt/modules/debian_ip.py | 2 +- salt/states/network.py | 87 ++++++++++++++++++++++++++++++++------- 2 files changed, 74 insertions(+), 15 deletions(-) diff --git a/salt/modules/debian_ip.py b/salt/modules/debian_ip.py index 2cee9e770bf1..5ed96003b012 100644 --- a/salt/modules/debian_ip.py +++ b/salt/modules/debian_ip.py @@ -1708,7 +1708,7 @@ def build_interface(iface, iface_type, enabled, **settings): settings['proto'] = 'static' if 'ipv6ipaddr' not in settings and 'ipv6ipaddrs' in settings: - settings['ipv6addr'] = settings['ipv6ipaddrs'].pop(0) + settings['ipv6ipaddr'] = settings['ipv6ipaddrs'].pop(0) if 'ipaddr' not in settings and 'ipaddrs' in settings: settings['ipaddr'] = settings['ipaddrs'].pop(0) diff --git a/salt/states/network.py b/salt/states/network.py index f09cb418bd56..038a21714eb7 100644 --- a/salt/states/network.py +++ b/salt/states/network.py @@ -9,9 +9,12 @@ .. note:: - Prior to version 2014.1.0, only RedHat-based systems (RHEL, - CentOS, Scientific Linux, etc.) are supported. Support for Debian/Ubuntu is - new in 2014.1.0 and should be considered experimental. + RedHat-based systems (RHEL, CentOS, Scientific, etc.) + have been supported since version 2014.1.0. + + Debian-based systems (Debian, Ubuntu, etc.) have been + supported since version 2017.7.0. The following options + are not supported: ipaddr_start, and ipaddr_end. Other platforms are not yet supported. @@ -31,9 +34,17 @@ network.managed: - enabled: True - type: eth - - proto: none - - ipaddr: 10.1.0.1 + - proto: static + - ipaddr: 10.1.0.7 - netmask: 255.255.255.0 + - gateway: 10.1.0.1 + - enable_ipv6: true + - ipv6proto: static + - ipv6ipaddrs: + - 2001:db8:dead:beef::3/64 + - 2001:db8:dead:beef::7/64 + - ipv6gateway: 2001:db8:dead:beef::1 + - ipv6netmask: 64 - dns: - 8.8.8.8 - 8.8.4.4 @@ -121,12 +132,11 @@ - type: bond - ipaddr: 10.1.0.1 - netmask: 255.255.255.0 - - mode: active-backup + - mode: gre - proto: static - dns: - 8.8.8.8 - 8.8.4.4 - - ipv6: - enabled: False - slaves: eth2 eth3 - require: @@ -202,6 +212,62 @@ - require: - network: eth4 + eth6: + network.managed: + - type: eth + - noifupdown: True + + # IPv4 + - proto: static + - ipaddr: 192.168.4.9 + - netmask: 255.255.255.0 + - gateway: 192.168.4.1 + - enable_ipv6: True + + # IPv6 + - ipv6proto: static + - ipv6ipaddr: 2001:db8:dead:c0::3 + - ipv6netmask: 64 + - ipv6gateway: 2001:db8:dead:c0::1 + # override shared; makes those options v4-only + - ipv6ttl: 15 + + # Shared + - mtu: 1480 + - ttl: 18 + - dns: + - 8.8.8.8 + - 8.8.4.4 + + eth7: + - type: eth + - proto: static + - ipaddr: 10.1.0.7 + - netmask: 255.255.255.0 + - gateway: 10.1.0.1 + - enable_ipv6: True + - ipv6proto: static + - ipv6ipaddr: 2001:db8:dead:beef::3 + - ipv6netmask: 64 + - ipv6gateway: 2001:db8:dead:beef::1 + - noifupdown: True + + eth8: + network.managed: + - enabled: True + - type: eth + - proto: static + - enable_ipv6: true + - ipv6proto: static + - ipv6ipaddrs: + - 2001:db8:dead:beef::3/64 + - 2001:db8:dead:beef::7/64 + - ipv6gateway: 2001:db8:dead:beef::1 + - ipv6netmask: 64 + - dns: + - 8.8.8.8 + - 8.8.4.4 + system: network.system: - enabled: True @@ -222,13 +288,6 @@ - userctl: no - ipv6_autoconf: no - enable_ipv6: true - - ipaddrs: - - 127.0.0.1/8 - - 10.1.0.4/32 - - 10.1.0.12/32 - - ipv6addrs: - - fc00::1/128 - - fc00::100/128 .. note:: Apply changes to hostname immediately. From 6e4122592a13bfd82afcf9c19f3070f23d9a73dd Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Tue, 28 Aug 2018 01:38:50 -0500 Subject: [PATCH 16/55] Finished adding support for multiple IP addresses. --- salt/modules/debian_ip.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/salt/modules/debian_ip.py b/salt/modules/debian_ip.py index 5ed96003b012..09030d866c3a 100644 --- a/salt/modules/debian_ip.py +++ b/salt/modules/debian_ip.py @@ -407,6 +407,7 @@ def __space_delimited_list(value): 'search': 'dns-search', 'hwaddr': 'hwaddress', # TODO: this limits bootp functionality 'ipaddr': 'address', + 'ipaddrs': 'addresses', } @@ -419,10 +420,11 @@ def __space_delimited_list(value): IPV4_VALID_PROTO = ['bootp', 'dhcp', 'static', 'manual', 'loopback', 'ppp'] -IPV4_ATTR_MAP = { +IPV6_ATTR_MAP = { 'proto': __within(IPV4_VALID_PROTO, dtype=six.text_type), # ipv4 static & manual 'address': __ipv4_quad, + 'addresses': __anything, 'netmask': __ipv4_netmask, 'broadcast': __ipv4_quad, 'metric': __int, @@ -473,6 +475,7 @@ def __space_delimited_list(value): 'proto': __within(IPV6_VALID_PROTO), # ipv6 static & manual 'address': __ipv6, + 'addresses': __anything, 'netmask': __ipv6_netmask, 'broadcast': __ipv6, 'gateway': __ipv6, # supports a colon-delimited list From 1b096fbee6abf80a45ccfe5917af5e9794c2735d Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Tue, 28 Aug 2018 02:07:29 -0500 Subject: [PATCH 17/55] Removed python lint. --- salt/modules/debian_ip.py | 2 +- tests/unit/modules/test_debian_ip.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/salt/modules/debian_ip.py b/salt/modules/debian_ip.py index 09030d866c3a..a0170ea7fb48 100644 --- a/salt/modules/debian_ip.py +++ b/salt/modules/debian_ip.py @@ -420,7 +420,7 @@ def __space_delimited_list(value): IPV4_VALID_PROTO = ['bootp', 'dhcp', 'static', 'manual', 'loopback', 'ppp'] -IPV6_ATTR_MAP = { +IPV4_ATTR_MAP = { 'proto': __within(IPV4_VALID_PROTO, dtype=six.text_type), # ipv4 static & manual 'address': __ipv4_quad, diff --git a/tests/unit/modules/test_debian_ip.py b/tests/unit/modules/test_debian_ip.py index 28a4d3249fee..fae672eba8ca 100644 --- a/tests/unit/modules/test_debian_ip.py +++ b/tests/unit/modules/test_debian_ip.py @@ -221,9 +221,9 @@ def test_build_interface(self): 'ipv6ipaddr': '2001:db8:dead:beef::3', 'ipv6netmask': '64', 'ipv6gateway': '2001:db8:dead:beef::1', - 'ttl': '18', # shared - 'ipv6ttl': '15', # overriden for v6 - 'mtu': '1480', # shared + 'ttl': '18', # shared + 'ipv6ttl': '15', # overriden for v6 + 'mtu': '1480', # shared 'enable_ipv6': True, 'noifupdown': True, }, @@ -441,9 +441,9 @@ def test_build_interface(self): # Skip tests that require __salt__['pkg.install']() if iface['iface_type'] not in ['bridge', 'pppoe', 'vlan']: self.assertListEqual(debian_ip.build_interface( - iface = iface['iface_name'], - iface_type = iface['iface_type'], - enabled = iface['enabled'], + iface=iface['iface_name'], + iface_type=iface['iface_type'], + enabled=iface['enabled'], **iface['settings']), iface['return']) From 0b5956115b4ebec31fb3bab8818cccf0e05a30e6 Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Tue, 28 Aug 2018 05:12:45 -0500 Subject: [PATCH 18/55] Added documentation about debian interfaces.d/*. (Fixes: #40262) --- salt/states/network.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/salt/states/network.py b/salt/states/network.py index 038a21714eb7..ec3c2fd8edf3 100644 --- a/salt/states/network.py +++ b/salt/states/network.py @@ -18,6 +18,21 @@ Other platforms are not yet supported. +.. note:: + + On Debian-based systems, networking configuration can be specified + in `/etc/network/interfaces` or via included files such as (by default) + `/etc/network/interfaces.d/*`. This can be problematic for configuration + management. It is recommended to use either `file.managed` *or* + `network.managed`. + + If using `network.managed`, it can be useful to ensure `interfaces.d/` + is empty. This can be done using: + + /etc/network/interfaces.d: + file.directory: + - clean: True + .. code-block:: yaml system: From 15c34fe7a29a9c20983d8a777ee51595b6fa8f91 Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Tue, 28 Aug 2018 18:39:08 -0500 Subject: [PATCH 19/55] Support unicode in space-delimited list; fixes unit tests in py2. --- salt/modules/debian_ip.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/debian_ip.py b/salt/modules/debian_ip.py index a0170ea7fb48..dfa982461ee0 100644 --- a/salt/modules/debian_ip.py +++ b/salt/modules/debian_ip.py @@ -393,7 +393,7 @@ def __within(within=None, errmsg=None, dtype=None): def __space_delimited_list(value): '''validate that a value contains one or more space-delimited values''' - if isinstance(value, str): + if isinstance(value, six.string_types): value = value.strip().split() if hasattr(value, '__iter__') and value != []: From 622c354683fba70f51898be90d62e56a69da4bad Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Tue, 28 Aug 2018 21:24:56 -0500 Subject: [PATCH 20/55] Support reading multiple addresses from interface files. --- salt/modules/debian_ip.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/salt/modules/debian_ip.py b/salt/modules/debian_ip.py index dfa982461ee0..39ceda547818 100644 --- a/salt/modules/debian_ip.py +++ b/salt/modules/debian_ip.py @@ -629,7 +629,12 @@ def _parse_interfaces(interface_files=None): attrname = attr (valid, value, errmsg) = _validate_interface_option( attr, valuestr, addrfam) - iface_dict[attrname] = value + if attrname == 'address' and 'address' in iface_dict: + if 'addresses' not in iface_dict: + iface_dict['addresses'] = [] + iface_dict['addresses'].append(value) + else: + iface_dict[attrname] = value elif attr in _REV_ETHTOOL_CONFIG_OPTS: if 'ethtool' not in iface_dict: @@ -1710,11 +1715,6 @@ def build_interface(iface, iface_type, enabled, **settings): if 'proto' not in settings: settings['proto'] = 'static' - if 'ipv6ipaddr' not in settings and 'ipv6ipaddrs' in settings: - settings['ipv6ipaddr'] = settings['ipv6ipaddrs'].pop(0) - if 'ipaddr' not in settings and 'ipaddrs' in settings: - settings['ipaddr'] = settings['ipaddrs'].pop(0) - if iface_type == 'slave': settings['slave'] = 'yes' if 'master' not in settings: From d34eadec0cebbcd40b46a2eccf8bf164c6d8daf5 Mon Sep 17 00:00:00 2001 From: Michael Lustfield Date: Wed, 29 Aug 2018 17:39:32 -0500 Subject: [PATCH 21/55] Use a temp file instead of /etc/network/interfaces for unit tests. --- tests/unit/modules/test_debian_ip.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/unit/modules/test_debian_ip.py b/tests/unit/modules/test_debian_ip.py index fae672eba8ca..a49cda9f70d0 100644 --- a/tests/unit/modules/test_debian_ip.py +++ b/tests/unit/modules/test_debian_ip.py @@ -5,6 +5,7 @@ # Import Python libs from __future__ import absolute_import, print_function, unicode_literals +import tempfile # Import Salt Testing Libs from tests.support.mixins import LoaderModuleMockMixin @@ -437,15 +438,18 @@ def test_build_interface(self): '\n']}, ] - for iface in interfaces: - # Skip tests that require __salt__['pkg.install']() - if iface['iface_type'] not in ['bridge', 'pppoe', 'vlan']: - self.assertListEqual(debian_ip.build_interface( - iface=iface['iface_name'], - iface_type=iface['iface_type'], - enabled=iface['enabled'], - **iface['settings']), - iface['return']) + with tempfile.NamedTemporaryFile(mode='r', delete=True) as tfile: + with patch('salt.modules.debian_ip._DEB_NETWORK_FILE', str(tfile.name)): + for iface in interfaces: + # Skip tests that require __salt__['pkg.install']() + if iface['iface_type'] not in ['bridge', 'pppoe', 'vlan']: + self.assertListEqual(debian_ip.build_interface( + iface=iface['iface_name'], + iface_type=iface['iface_type'], + enabled=iface['enabled'], + interface_file=tfile.name, + **iface['settings']), + iface['return']) # 'up' function tests: 1 From 00cbf26d5ee85f1f59dcd2f0e63eb7cba149c458 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 6 Dec 2018 18:17:47 -0700 Subject: [PATCH 22/55] Add the netsh mechanism to the lgpo module Creates a new salt util for netsh and lgpo Adds the netsh mechanism to the lgpo module Adds additional functions to the firewall module --- salt/modules/win_firewall.py | 414 ++++++++++++++++++++++++- salt/modules/win_lgpo.py | 113 ++++++- salt/utils/win_lgpo_netsh.py | 574 +++++++++++++++++++++++++++++++++++ 3 files changed, 1098 insertions(+), 3 deletions(-) create mode 100644 salt/utils/win_lgpo_netsh.py diff --git a/salt/modules/win_firewall.py b/salt/modules/win_firewall.py index c80c933f704c..6d13dc1b6024 100644 --- a/salt/modules/win_firewall.py +++ b/salt/modules/win_firewall.py @@ -10,6 +10,7 @@ # Import Salt libs import salt.utils.platform from salt.exceptions import CommandExecutionError +import salt.utils.win_lgpo_netsh # Define the module's virtual name __virtualname__ = 'firewall' @@ -285,7 +286,7 @@ def delete_rule(name=None, salt '*' firewall.delete_rule 'test' '8080' 'tcp' 'in' # Delete the incoming tcp port 8000 from 192.168.0.1 in the rule named - # 'test_remote_ip` + # 'test_remote_ip' salt '*' firewall.delete_rule 'test_remote_ip' '8000' 'tcp' 'in' '192.168.0.1' # Delete all rules for local port 80: @@ -342,3 +343,414 @@ def rule_exists(name): return True except CommandExecutionError: return False + + +def get_settings(profile, section, store='local'): + ''' + Get the firewall property from the specified profile in the specified store + as returned by ``netsh advfirewall``. + + Args: + + profile (str): + The firewall profile to query. Valid options are: + + - domain + - public + - private + + section (str): + The property to query within the selected profile. Valid options + are: + + - firewallpolicy : inbound/outbound behavior + - logging : firewall logging settings + - settings : firewall properties + - state : firewalls state (on | off) + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + dict: A dictionary containing the properties for the specified profile + + Raises: + CommandExecutionError: If an error occurs + ValueError: If the parameters are incorrect + + CLI Example: + + .. code-block:: bash + + # Get the inbound/outbound firewall settings for connections on the + # local domain profile + salt * win_firewall.get_settings domain firewallpolicy + + # Get the inbound/outbound firewall settings for connections on the + # domain profile as defined by local group policy + salt * win_firewall.get_settings domain firewallpolicy lgpo + ''' + return salt.utils.win_lgpo_netsh.get_settings(profile=profile, + section=section, + store=store) + + +def get_all_settings(domain, store='local'): + ''' + Gets all the properties for the specified profile in the specified store + + Args: + + profile (str): + The firewall profile to query. Valid options are: + + - domain + - public + - private + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + dict: A dictionary containing the specified settings + + CLI Example: + + .. code-block:: bash + + # Get all firewall settings for connections on the domain profile + salt * win_firewall.get_all_settings domain + + # Get all firewall settings for connections on the domain profile as + # defined by local group policy + salt * win_firewall.get_all_settings domain lgpo + ''' + return salt.utils.win_lgpo_netsh.get_all_settings(domain, store) + + +def get_all_profiles(store='local'): + ''' + Gets all properties for all profiles in the specified store + + Args: + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + dict: A dictionary containing the specified settings for each profile + + CLI Example: + + .. code-block:: bash + + # Get all firewall settings for all profiles + salt * firewall.get_all_settings + + # Get all firewall settings for all profiles as defined by local group + # policy + + salt * firewall.get_all_settings lgpo + ''' + return salt.utils.win_lgpo_netsh.get_all_profiles(store=store) + + +def set_firewall_settings(profile, inbound=None, outbound=None, store='local'): + ''' + Set the firewall inbound/outbound settings for the specified profile and + store + + Args: + + profile (str): + The firewall profile to query. Valid options are: + + - domain + - public + - private + + inbound (str): + The inbound setting. If ``None`` is passed, the setting will remain + unchanged. Valid values are: + + - blockinbound + - blockinboundalways + - allowinbound + - notconfigured + + Default is ``None`` + + outbound (str): + The outbound setting. If ``None`` is passed, the setting will remain + unchanged. Valid values are: + + - allowoutbound + - blockoutbound + - notconfigured + + Default is ``None`` + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + bool: ``True`` if successful + + Raises: + CommandExecutionError: If an error occurs + ValueError: If the parameters are incorrect + + CLI Example: + + .. code-block:: bash + + # Set the inbound setting for the domain profile to block inbound + # connections + salt * firewall.set_firewall_settings domain='domain' inbound='blockinbound' + + # Set the outbound setting for the domain profile to allow outbound + # connections + salt * firewall.set_firewall_settings domain='domain' outbound='allowoutbound' + + # Set inbound/outbound settings for the domain profile in the group + # policy to block inbound and allow outbound + salt * firewall.set_firewall_settings domain='domain' inbound='blockinbound' outbound='allowoutbound' store='lgpo' + ''' + return salt.utils.win_lgpo_netsh.set_firewall_settings(profile=profile, + inbound=inbound, + outbound=outbound, + store=store) + + +def set_logging_settings(profile, setting, value, store='local'): + ''' + Configure logging settings for the Windows firewall. + + Args: + + profile (str): + The firewall profile to configure. Valid options are: + + - domain + - public + - private + + setting (str): + The logging setting to configure. Valid options are: + + - allowedconnections + - droppedconnections + - filename + - maxfilesize + + value (str): + The value to apply to the setting. Valid values are dependent upon + the setting being configured. Valid options are: + + allowedconnections: + + - enable + - disable + - notconfigured + + droppedconnections: + + - enable + - disable + - notconfigured + + filename: + + - Full path and name of the firewall log file + - notconfigured + + maxfilesize: + + - 1 - 32767 + - notconfigured + + .. note:: + ``notconfigured`` can only be used when using the lgpo store + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + bool: ``True`` if successful + + Raises: + CommandExecutionError: If an error occurs + ValueError: If the parameters are incorrect + + CLI Example: + + .. code-block:: bash + + # Log allowed connections and set that in local group policy + salt * firewall.set_logging_settings domain allowedconnections enable lgpo + + # Don't log dropped connections + salt * firewall.set_logging_settings profile=private setting=droppedconnections value=disable + + # Set the location of the log file + salt * firewall.set_logging_settings domain filename C:\windows\logs\firewall.log + + # You can also use environment variables + salt * firewall.set_logging_settings domain filename %systemroot%\system32\LogFiles\Firewall\pfirewall.log + + # Set the max file size of the log to 2048 Kb + salt * firewall.set_logging_settings domain maxfilesize 2048 + ''' + return salt.utils.win_lgpo_netsh.set_logging_settings(profile=profile, + setting=setting, + value=value, + store=store) + + +def set_settings(profile, setting, value, store='local'): + ''' + Configure firewall settings. + + Args: + + profile (str): + The firewall profile to configure. Valid options are: + + - domain + - public + - private + + setting (str): + The firewall setting to configure. Valid options are: + + - localfirewallrules + - localconsecrules + - inboundusernotification + - remotemanagement + - unicastresponsetomulticast + + value (str): + The value to apply to the setting. Valid options are + + - enable + - disable + - notconfigured + + .. note:: + ``notconfigured`` can only be used when using the lgpo store + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + bool: ``True`` if successful + + Raises: + CommandExecutionError: If an error occurs + ValueError: If the parameters are incorrect + + CLI Example: + + .. code-block:: bash + + # Merge local rules with those distributed through group policy + salt * firewall.set_settings domain localfirewallrules enable + + # Allow remote management of Windows Firewall + salt * firewall.set_settings domain remotemanagement enable + ''' + return salt.utils.win_lgpo_netsh.set_settings(profile=profile, + setting=setting, + value=value, + store=store) + + +def set_state(profile, state, store='local'): + ''' + Configure the firewall state. + + Args: + + profile (str): + The firewall profile to configure. Valid options are: + + - domain + - public + - private + + state (str): + The firewall state. Valid options are: + + - on + - off + - notconfigured + + .. note:: + ``notconfigured`` can only be used when using the lgpo store + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + bool: ``True`` if successful + + Raises: + CommandExecutionError: If an error occurs + ValueError: If the parameters are incorrect + + CLI Example: + + .. code-block:: bash + + # Turn the firewall off when the domain profile is active + salt * firewall.set_state domain off + + # Turn the firewall on when the public profile is active and set that in + # the local group policy + salt * firewall.set_state public on lgpo + ''' + return salt.utils.win_lgpo_netsh.set_state(profile=profile, + state=state, + store=store) diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py index f594bc5f3b48..8364499a6163 100644 --- a/salt/modules/win_lgpo.py +++ b/salt/modules/win_lgpo.py @@ -55,6 +55,7 @@ import salt.utils.path import salt.utils.platform import salt.utils.stringutils +import salt.utils.win_lgpo_netsh # Import 3rd-party libs from salt.ext import six @@ -224,7 +225,7 @@ class _policy_info(object): Access" ======= =================================================================== - LsaRights mechanism + LsaRights Mechanism ------------------- LSA Rights policies are configured via the LsaRights mechanism. The value of @@ -237,7 +238,7 @@ class _policy_info(object): **SeNetworkLogonRight** ====== ==================================================================== - NetUserModal mechanism + NetUserModal Mechanism ---------------------- Some policies are configurable by the **NetUserModalGet** and @@ -254,6 +255,34 @@ class _policy_info(object): policy, for example **max_passwd_age** ====== ==================================================================== + NetSH Mechanism + --------------- + + The firewall policies are configured by the ``netsh.exe`` executable. The + value of this key is a dict with the following make-up: + + ======= =================================================================== + Key Value + ======= =================================================================== + Profile The firewall profile to modify. Can be one of Domain, Private, or + Public + Section The section of the firewall to modify. Can be one of state, + firewallpolicy, settings, or logging. + Option The setting within that section + Value The value of the setting + ======= =================================================================== + + More information can be found in the advfirewall context in netsh. This can + be access by opening a netsh prompt. At a command prompt type the following: + + c:\>netsh + netsh>advfirewall + netsh advfirewall>set help + netsh advfirewall>set domain help + + Transforms + ---------- + Optionally, each policy definition can contain a "Transform" key. The Transform key is used to handle data that is stored and viewed differently. This key's value is a dict with the following key/value pairs: @@ -368,6 +397,13 @@ def __init__(self): 'Local Policies', 'Security Options' ] + self.windows_firewall_gpedit_path = [ + 'Computer Configuration', + 'Windows Settings', + 'Security Settings', + 'Windows Firewall with Advanced Security', + 'Windows Firewall with Advanced Security - Local Group Policy Object' + ] self.password_policy_gpedit_path = [ 'Computer Configuration', 'Windows Settings', @@ -436,6 +472,12 @@ def __init__(self): None: 'Not Defined', '(value not set)': 'Not Defined' } + self.firewall_connections = { + 'blockinbound': 'Block (default)', + 'blockinboundalways': 'Block all connections', + 'allowinbound': 'Allow', + 'notconfigured': 'Not Configured' + } self.krb_encryption_types = { 0: 'No minimum', 1: 'DES_CBC_CRC', @@ -817,6 +859,33 @@ def __init__(self): }, }, }, + 'wfw_DomainInboundConnections': { + 'Policy': 'Network firewall: Domain: Inbound connections', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Block (default) + # - Block all connections + # - Allow + # - Not Configured + 'Settings': self.firewall_connections.keys(), + 'NetSH': { + 'Profile': 'domain', + 'Section': 'firewallpolicy', + 'Option': 'Inbound' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_connections, + 'value_lookup': True, + }, + }, + }, 'PasswordHistory': { 'Policy': 'Enforce password history', 'lgpo_section': self.password_policy_gpedit_path, @@ -3571,6 +3640,23 @@ def _buildElementNsmap(using_elements): return thisMap +def _findOptionValueNetSH(profile, option): + settings = salt.utils.win_lgpo_netsh.get_all_settings(profile=profile, + store='lgpo') + return settings[option] + + +def _setOptionValueNetSH(profile, section, option, value): + if section == 'firewallpolicy': + if option not in ('Inbound', 'Outbound'): + raise CommandExecutionError('Invalid option: {0}'.format(option)) + return salt.utils.win_lgpo_netsh.set_firewall_settings( + profile=profile, + inbound=value if option == 'Inbound' else None, + outbound=value if option == 'Outbound' else None, + store='lgpo') + + def _findOptionValueInSeceditFile(option): ''' helper function to dump/parse a `secedit /export` file for a particular option @@ -5939,6 +6025,12 @@ def get(policy_class=None, return_full_policy_names=True, else: msg = 'An error occurred attempting to get the value of policy {0} from secedit' raise CommandExecutionError(msg.format(policy_name)) + elif 'NetSH' in _pol: + # get value from netsh + class_vals[policy_name] = _findOptionValueNetSH( + profile=_pol['NetSH']['Profile'], + option=_pol['NetSH']['Option']) + elif 'NetUserModal' in _pol: # get value from UserNetMod if _pol['NetUserModal']['Modal'] not in modal_returns: @@ -6160,6 +6252,7 @@ class in this module. if policies: for p_class in policies: _secedits = {} + _netshs = {} _modal_sets = {} _admTemplateData = {} _regedits = {} @@ -6199,6 +6292,15 @@ class in this module. _secedits[_pol['Secedit']['Section']].append( ' '.join([_pol['Secedit']['Option'], '=', six.text_type(_value)])) + elif 'NetSH' in _pol: + # set value with netsh + log.debug('%s is a NetSH policy', policy_name) + _netshs.setdefault(policy_name, { + 'profile': _pol['NetSH']['Profile'], + 'section': _pol['NetSH']['Section'], + 'option': _pol['NetSH']['Option'], + 'value': six.text_type(_value) + }) elif 'NetUserModal' in _pol: # set value via NetUserModal log.debug('%s is a NetUserModal policy', policy_name) @@ -6388,6 +6490,13 @@ class in this module. msg = ('Error while attempting to set policies via secedit.' ' Some changes may not be applied as expected') raise CommandExecutionError(msg) + if _netshs: + # we've got netsh settings to make + for setting in _netshs: + log.debug('Setting firewall policy: {0}'.format(setting)) + log.debug(_netshs[setting]) + _setOptionValueNetSH(**_netshs[setting]) + if _modal_sets: # we've got modalsets to make log.debug(_modal_sets) diff --git a/salt/utils/win_lgpo_netsh.py b/salt/utils/win_lgpo_netsh.py new file mode 100644 index 000000000000..d1d2a67b3fbf --- /dev/null +++ b/salt/utils/win_lgpo_netsh.py @@ -0,0 +1,574 @@ +# -*- coding: utf-8 -*- +''' +A salt util for modifying firewall settings. + +This util allows you to modify firewall settings in the local group policy in +addition to the normal firewall settings. Parameters are taken from the +netsh advfirewall prompt. + +.. note:: + More information can be found in the advfirewall context in netsh. This can + be access by opening a netsh prompt. At a command prompt type the following: + + c:\>netsh + netsh>advfirewall + netsh advfirewall>set help + netsh advfirewall>set domain help + +Usage: + +.. code-block:: python + + import salt.utils.win_lgpo_netsh + + # Get the inbound/outbound firewall settings for connections on the + # local domain profile + salt.utils.win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy') + + # Get the inbound/outbound firewall settings for connections on the + # domain profile as defined by local group policy + salt.utils.win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy', + store='lgpo') + + # Get all firewall settings for connections on the domain profile + salt.utils.win_lgpo_netsh.get_all_settings(profile='domain') + + # Get all firewall settings for connections on the domain profile as + # defined by local group policy + salt.utils.win_lgpo_netsh.get_all_settings(profile='domain', store='lgpo') + + # Get all firewall settings for all profiles + salt.utils.win_lgpo_netsh.get_all_settings() + + # Get all firewall settings for all profiles as defined by local group + # policy + salt.utils.win_lgpo_netsh.get_all_settings(store='lgpo') + + # Set the inbound setting for the domain profile to block inbound + # connections + salt.utils.win_lgpo_netsh.set_firewall_settings(profile='domain', + inbound='blockinbound') + + # Set the outbound setting for the domain profile to allow outbound + # connections + salt.utils.win_lgpo_netsh.set_firewall_settings(profile='domain', + outbound='allowoutbound') + + # Set inbound/outbound settings for the domain profile in the group + # policy to block inbound and allow outbound + salt.utils.win_lgpo_netsh.set_firewall_settings(profile='domain', + inbound='blockinbound', + outbound='allowoutbound', + store='lgpo') +''' +# Import Python libs +from __future__ import absolute_import, unicode_literals, print_function +from textwrap import dedent +import logging +import os +import re +import socket +import tempfile + +import salt.modules.cmdmod +from salt.exceptions import CommandExecutionError + +log = logging.getLogger(__name__) +__hostname__ = socket.gethostname() + + +def _netsh_file(content): + ''' + helper function to get the results of ``netsh -f content.txt`` + + Running ``netsh`` will drop you into a ``netsh`` prompt where you can issue + ``netsh`` commands. You can put a series of commands in an external file and + run them as if from a ``netsh`` prompt using the ``-f`` switch. That's what + this function does. + + Args: + + content (str): + The contents of the file that will be run by the ``netsh -f`` + command + + Returns: + str: The text returned by the netsh command + ''' + with tempfile.NamedTemporaryFile(mode='w', + prefix='salt-', + suffix='.netsh', + delete=False) as fp: + fp.write(content) + try: + log.debug('{0}:\n{1}'.format(fp.name, content)) + return salt.modules.cmdmod.run('netsh -f {0}'.format(fp.name), python_shell=True) + finally: + os.remove(fp.name) + + +def _netsh_command(command, store): + if store.lower() not in ('local', 'lgpo'): + raise ValueError('Incorrect store: {0}'.format(store)) + # set the store for local or lgpo + if store.lower() == 'local': + netsh_script = dedent('''\ + advfirewall + set store local + {0} + '''.format(command)) + else: + netsh_script = dedent('''\ + advfirewall + set store gpo = {0} + {1} + '''.format(__hostname__, command)) + return _netsh_file(content=netsh_script).splitlines() + + +def get_settings(profile, section, store='local'): + ''' + Get the firewall property from the specified profile in the specified store + as returned by ``netsh advfirewall``. + + Args: + + profile (str): + The firewall profile to query. Valid options are: + + - domain + - public + - private + + section (str): + The property to query within the selected profile. Valid options + are: + + - firewallpolicy : inbound/outbound behavior + - logging : firewall logging settings + - settings : firewall properties + - state : firewalls state (on | off) + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + dict: A dictionary containing the properties for the specified profile + + Raises: + CommandExecutionError: If an error occurs + ValueError: If the parameters are incorrect + ''' + # validate input + if profile.lower() not in ('domain', 'public', 'private'): + raise ValueError('Incorrect profile: {0}'.format(profile)) + if section.lower() not in ('state', 'firewallpolicy', 'settings', 'logging'): + raise ValueError('Incorrect section: {0}'.format(section)) + if store.lower() not in ('local', 'lgpo'): + raise ValueError('Incorrect store: {0}'.format(store)) + command = 'show {0}profile {1}'.format(profile, section) + # run it + results = _netsh_command(command=command, store=store) + # sample output: + # Domain Profile Settings: + # ---------------------------------------------------------------------- + # LocalFirewallRules N/A (GPO-store only) + # LocalConSecRules N/A (GPO-store only) + # InboundUserNotification Disable + # RemoteManagement Disable + # UnicastResponseToMulticast Enable + + # if it's less than 3 lines it failed + if len(results) < 3: + raise CommandExecutionError('Invalid results: {0}'.format(results)) + ret = {} + # Skip the first 2 lines. Add everything else to a dictionary + for line in results[3:]: + ret.update(dict(map(None, *[iter(re.split(r"\s{2,}", line))]*2))) + + # Remove spaces from the values so that `Not Configured` is detected + # correctly + for item in ret: + ret[item] = ret[item].replace(' ', '') + + # special handling for firewallpolicy + if section == 'firewallpolicy': + inbound, outbound = ret['Firewall Policy'].split(',') + return {'Inbound': inbound, 'Outbound': outbound} + + return ret + + +def get_all_settings(profile, store='local'): + ''' + Gets all the properties for the specified profile in the specified store + + Args: + + profile (str): + The firewall profile to query. Valid options are: + + - domain + - public + - private + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + dict: A dictionary containing the specified settings + ''' + ret = dict() + ret.update(get_settings(profile=profile, section='state', store=store)) + ret.update(get_settings(profile=profile, section='firewallpolicy', store=store)) + ret.update(get_settings(profile=profile, section='settings', store=store)) + ret.update(get_settings(profile=profile, section='logging', store=store)) + return ret + + +def get_all_profiles(store='local'): + ''' + Gets all properties for all profiles in the specified store + + Args: + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + dict: A dictionary containing the specified settings for each profile + ''' + return { + 'Domain Profile': get_all_settings(profile='domain', store=store), + 'Private Profile': get_all_settings(profile='private', store=store), + 'Public Profile': get_all_settings(profile='public', store=store) + } + + +def set_firewall_settings(profile, + inbound=None, + outbound=None, + store='local'): + ''' + Set the firewall inbound/outbound settings for the specified profile and + store + + Args: + + profile (str): + The firewall profile to configure. Valid options are: + + - domain + - public + - private + + inbound (str): + The inbound setting. If ``None`` is passed, the setting will remain + unchanged. Valid values are: + + - blockinbound + - blockinboundalways + - allowinbound + - notconfigured + + Default is ``None`` + + outbound (str): + The outbound setting. If ``None`` is passed, the setting will remain + unchanged. Valid values are: + + - allowoutbound + - blockoutbound + - notconfigured + + Default is ``None`` + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + bool: ``True`` if successful + + Raises: + CommandExecutionError: If an error occurs + ValueError: If the parameters are incorrect + ''' + # Input validation + if profile.lower() not in ('domain', 'public', 'private'): + raise ValueError('Incorrect profile: {0}'.format(profile)) + if inbound and inbound.lower() not in ('blockinbound', + 'blockinboundalways', + 'allowinbound', + 'notconfigured'): + raise ValueError('Incorrect inbound value: {0}'.format(inbound)) + if outbound and outbound.lower() not in ('allowoutbound', + 'blockoutbound', + 'notconfigured'): + raise ValueError('Incorrect outbound value: {0}'.format(outbound)) + if not inbound and not outbound: + raise ValueError('Must set inbound or outbound') + + # You have to specify inbound and outbound setting at the same time + # If you're only specifying one, you have to get the current setting for the + # other + if not inbound or not outbound: + ret = get_settings(profile=profile, + section='firewallpolicy', + store=store) + if not inbound: + inbound = ret['Inbound'] + if not outbound: + outbound = ret['Outbound'] + + command = 'set {0}profile firewallpolicy {1},{2}' \ + ''.format(profile, inbound, outbound) + + results = _netsh_command(command=command, store=store) + + if results: + raise CommandExecutionError('An error occurred: {0}'.format(results)) + + return True + + +def set_logging_settings(profile, setting, value, store='local'): + ''' + Configure logging settings for the Windows firewall. + + Args: + + profile (str): + The firewall profile to configure. Valid options are: + + - domain + - public + - private + + setting (str): + The logging setting to configure. Valid options are: + + - allowedconnections + - droppedconnections + - filename + - maxfilesize + + value (str): + The value to apply to the setting. Valid values are dependent upon + the setting being configured. Valid options are: + + allowedconnections: + + - enable + - disable + - notconfigured + + droppedconnections: + + - enable + - disable + - notconfigured + + filename: + + - Full path and name of the firewall log file + - notconfigured + + maxfilesize: + + - 1 - 32767 (Kb) + - notconfigured + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + bool: ``True`` if successful + + Raises: + CommandExecutionError: If an error occurs + ValueError: If the parameters are incorrect + ''' + # Input validation + if profile.lower() not in ('domain', 'public', 'private'): + raise ValueError('Incorrect profile: {0}'.format(profile)) + if setting.lower() not in ('allowedconnections', + 'droppedconnections', + 'filename', + 'maxfilesize'): + raise ValueError('Incorrect setting: {0}'.format(setting)) + if setting.lower() in ('allowedconnections', 'droppedconnections'): + if value.lower() not in ('enable', 'disable', 'notconfigured'): + raise ValueError('Incorrect value: {0}'.format(value)) + # TODO: Consider adding something like the following to validate filename + # https://stackoverflow.com/questions/9532499/check-whether-a-path-is-valid-in-python-without-creating-a-file-at-the-paths-ta + if setting.lower() == 'maxfilesize': + if value.lower() != 'notconfigured': + # Must be a number between 1 and 32767 + try: + int(value) + except ValueError: + raise ValueError('Incorrect value: {0}'.format(value)) + if not 1 >= int(value) <= 32767: + raise ValueError('Incorrect value: {0}'.format(value)) + # Run the command + command = 'set {0}profile logging {1} {2}'.format(profile, setting, value) + results = _netsh_command(command=command, store=store) + + # A successful run should return an empty list + if results: + raise CommandExecutionError('An error occurred: {0}'.format(results)) + + return True + + +def set_settings(profile, setting, value, store='local'): + ''' + Configure firewall settings. + + Args: + + profile (str): + The firewall profile to configure. Valid options are: + + - domain + - public + - private + + setting (str): + The firewall setting to configure. Valid options are: + + - localfirewallrules + - localconsecrules + - inboundusernotification + - remotemanagement + - unicastresponsetomulticast + + value (str): + The value to apply to the setting. Valid options are + + - enable + - disable + - notconfigured + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + bool: ``True`` if successful + + Raises: + CommandExecutionError: If an error occurs + ValueError: If the parameters are incorrect + ''' + # Input validation + if profile.lower() not in ('domain', 'public', 'private'): + raise ValueError('Incorrect profile: {0}'.format(profile)) + if setting not in ('localfirewallrules', + 'localconsecrules', + 'inboundusernotification', + 'remotemanagement', + 'unicastresponsetomulticast'): + raise ValueError('Incorrect setting: {0}'.format(setting)) + if value not in ('enable', 'disable', 'notconfigured'): + raise ValueError('Incorrect value: {0}'.format(value)) + + # Run the command + command = 'set {0}profile settings {1} {2}'.format(profile, setting, value) + results = _netsh_command(command=command, store=store) + + # A successful run should return an empty list + if results: + raise CommandExecutionError('An error occurred: {0}'.format(results)) + + return True + + +def set_state(profile, state, store='local'): + ''' + Configure the firewall state. + + Args: + + profile (str): + The firewall profile to configure. Valid options are: + + - domain + - public + - private + + state (str): + The firewall state. Valid options are: + + - on + - off + - notconfigured + + store (str): + The store to use. This is either the local firewall policy or the + policy defined by local group policy. Valid options are: + + - lgpo + - local + + Default is ``local`` + + Returns: + bool: ``True`` if successful + + Raises: + CommandExecutionError: If an error occurs + ValueError: If the parameters are incorrect + ''' + # Input validation + if profile.lower() not in ('domain', 'public', 'private'): + raise ValueError('Incorrect profile: {0}'.format(profile)) + if state.lower() not in ('on', 'off', 'notconfigured'): + raise ValueError('Incorrect state: {0}'.format(state)) + + # Run the command + command = 'set {0}profile state {1}'.format(profile, state) + results = _netsh_command(command=command, store=store) + + # A successful run should return an empty list + if results: + raise CommandExecutionError('An error occurred: {0}'.format(results)) + + return True From cb50648715d639f398dfb747e7bd1aabffaa2094 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 6 Dec 2018 18:23:39 -0700 Subject: [PATCH 23/55] Add versionadded tags --- salt/modules/win_firewall.py | 21 +++++++++++++++++++++ salt/utils/win_lgpo_netsh.py | 3 +++ 2 files changed, 24 insertions(+) diff --git a/salt/modules/win_firewall.py b/salt/modules/win_firewall.py index 6d13dc1b6024..5ebd5207168c 100644 --- a/salt/modules/win_firewall.py +++ b/salt/modules/win_firewall.py @@ -350,6 +350,9 @@ def get_settings(profile, section, store='local'): Get the firewall property from the specified profile in the specified store as returned by ``netsh advfirewall``. + .. versionadded:: 2018.3.4 + .. versionadded:: Fluorine + Args: profile (str): @@ -405,6 +408,9 @@ def get_all_settings(domain, store='local'): ''' Gets all the properties for the specified profile in the specified store + .. versionadded:: 2018.3.4 + .. versionadded:: Fluorine + Args: profile (str): @@ -444,6 +450,9 @@ def get_all_profiles(store='local'): ''' Gets all properties for all profiles in the specified store + .. versionadded:: 2018.3.4 + .. versionadded:: Fluorine + Args: store (str): @@ -478,6 +487,9 @@ def set_firewall_settings(profile, inbound=None, outbound=None, store='local'): Set the firewall inbound/outbound settings for the specified profile and store + .. versionadded:: 2018.3.4 + .. versionadded:: Fluorine + Args: profile (str): @@ -550,6 +562,9 @@ def set_logging_settings(profile, setting, value, store='local'): ''' Configure logging settings for the Windows firewall. + .. versionadded:: 2018.3.4 + .. versionadded:: Fluorine + Args: profile (str): @@ -641,6 +656,9 @@ def set_settings(profile, setting, value, store='local'): ''' Configure firewall settings. + .. versionadded:: 2018.3.4 + .. versionadded:: Fluorine + Args: profile (str): @@ -705,6 +723,9 @@ def set_state(profile, state, store='local'): ''' Configure the firewall state. + .. versionadded:: 2018.3.4 + .. versionadded:: Fluorine + Args: profile (str): diff --git a/salt/utils/win_lgpo_netsh.py b/salt/utils/win_lgpo_netsh.py index d1d2a67b3fbf..c569d46003e3 100644 --- a/salt/utils/win_lgpo_netsh.py +++ b/salt/utils/win_lgpo_netsh.py @@ -2,6 +2,9 @@ ''' A salt util for modifying firewall settings. +.. versionadded:: 2018.3.4 +.. versionadded:: Fluorine + This util allows you to modify firewall settings in the local group policy in addition to the normal firewall settings. Parameters are taken from the netsh advfirewall prompt. From f18d1aa8901f4b5377e40d3f520b3d98cd91fbed Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 7 Dec 2018 11:00:15 -0700 Subject: [PATCH 24/55] Add additional policies that use netsh --- salt/modules/win_lgpo.py | 110 ++++++++++++++++++++++++++++++++--- salt/utils/win_lgpo_netsh.py | 12 ++-- 2 files changed, 107 insertions(+), 15 deletions(-) diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py index 8364499a6163..765169228495 100644 --- a/salt/modules/win_lgpo.py +++ b/salt/modules/win_lgpo.py @@ -472,11 +472,21 @@ def __init__(self): None: 'Not Defined', '(value not set)': 'Not Defined' } - self.firewall_connections = { + self.firewall_inbound_connections = { 'blockinbound': 'Block (default)', 'blockinboundalways': 'Block all connections', 'allowinbound': 'Allow', - 'notconfigured': 'Not Configured' + 'notconfigured': 'Not configured' + } + self.firewall_outbound_connections = { + 'blockoutbound': 'Block', + 'allowoutbound': 'Allow (default)', + 'notconfigured': 'Not configured' + } + self.firewall_rule_merging = { + 'enable': 'Yes (default)', + 'disable': 'No', + 'notconfigured': 'Not configured' } self.krb_encryption_types = { 0: 'No minimum', @@ -859,15 +869,15 @@ def __init__(self): }, }, }, - 'wfw_DomainInboundConnections': { + 'WfwDomainInboundConnections': { 'Policy': 'Network firewall: Domain: Inbound connections', 'lgpo_section': self.windows_firewall_gpedit_path, # Settings available are: # - Block (default) # - Block all connections # - Allow - # - Not Configured - 'Settings': self.firewall_connections.keys(), + # - Not configured + 'Settings': self.firewall_inbound_connections.keys(), 'NetSH': { 'Profile': 'domain', 'Section': 'firewallpolicy', @@ -877,11 +887,89 @@ def __init__(self): 'Get': '_dict_lookup', 'Put': '_dict_lookup', 'GetArgs': { - 'lookup': self.firewall_connections, + 'lookup': self.firewall_inbound_connections, 'value_lookup': False, }, 'PutArgs': { - 'lookup': self.firewall_connections, + 'lookup': self.firewall_inbound_connections, + 'value_lookup': True, + }, + }, + }, + 'WfwDomainOutboundConnections': { + 'Policy': 'Network firewall: Domain: Outbound connections', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Block + # - Allow (default) + # - Not configured + 'Settings': self.firewall_outbound_connections.keys(), + 'NetSH': { + 'Profile': 'domain', + 'Section': 'firewallpolicy', + 'Option': 'Outbound' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_outbound_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_outbound_connections, + 'value_lookup': True, + }, + }, + }, + 'WfwDomainSettingsLocalFirewallRules': { + 'Policy': 'Network firewall: Domain: Settings: Apply local firewall rules', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes (default) + # - No + # - Not configured + 'Settings': self.firewall_rule_merging.keys(), + 'NetSH': { + 'Profile': 'domain', + 'Section': 'settings', + 'Option': 'LocalFirewallRules' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': True, + }, + }, + }, + 'WfwDomainSettingsLocalConnectionRules': { + 'Policy': 'Network firewall: Domain: Settings: Apply local connection security rules', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes (default) + # - No + # - Not configured + 'Settings': self.firewall_rule_merging.keys(), + 'NetSH': { + 'Profile': 'domain', + 'Section': 'settings', + 'Option': 'LocalConSecRules' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_rule_merging, 'value_lookup': True, }, }, @@ -3648,13 +3736,17 @@ def _findOptionValueNetSH(profile, option): def _setOptionValueNetSH(profile, section, option, value): if section == 'firewallpolicy': - if option not in ('Inbound', 'Outbound'): - raise CommandExecutionError('Invalid option: {0}'.format(option)) return salt.utils.win_lgpo_netsh.set_firewall_settings( profile=profile, inbound=value if option == 'Inbound' else None, outbound=value if option == 'Outbound' else None, store='lgpo') + if section == 'settings': + return salt.utils.win_lgpo_netsh.set_settings( + profile=profile, + setting=option, + value=value, + store='lgpo') def _findOptionValueInSeceditFile(option): diff --git a/salt/utils/win_lgpo_netsh.py b/salt/utils/win_lgpo_netsh.py index c569d46003e3..c548b8d24ca9 100644 --- a/salt/utils/win_lgpo_netsh.py +++ b/salt/utils/win_lgpo_netsh.py @@ -504,13 +504,13 @@ def set_settings(profile, setting, value, store='local'): # Input validation if profile.lower() not in ('domain', 'public', 'private'): raise ValueError('Incorrect profile: {0}'.format(profile)) - if setting not in ('localfirewallrules', - 'localconsecrules', - 'inboundusernotification', - 'remotemanagement', - 'unicastresponsetomulticast'): + if setting.lower() not in ('localfirewallrules', + 'localconsecrules', + 'inboundusernotification', + 'remotemanagement', + 'unicastresponsetomulticast'): raise ValueError('Incorrect setting: {0}'.format(setting)) - if value not in ('enable', 'disable', 'notconfigured'): + if value.lower() not in ('enable', 'disable', 'notconfigured'): raise ValueError('Incorrect value: {0}'.format(value)) # Run the command From b2b97959d0d840c425175ebe9665d71fa1247252 Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 7 Dec 2018 13:21:37 -0700 Subject: [PATCH 25/55] Fix overly long lines --- salt/modules/win_lgpo.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py index 765169228495..45858c5c1023 100644 --- a/salt/modules/win_lgpo.py +++ b/salt/modules/win_lgpo.py @@ -923,7 +923,8 @@ def __init__(self): }, }, 'WfwDomainSettingsLocalFirewallRules': { - 'Policy': 'Network firewall: Domain: Settings: Apply local firewall rules', + 'Policy': 'Network firewall: Domain: Settings: Apply ' + 'local firewall rules', 'lgpo_section': self.windows_firewall_gpedit_path, # Settings available are: # - Yes (default) @@ -949,7 +950,8 @@ def __init__(self): }, }, 'WfwDomainSettingsLocalConnectionRules': { - 'Policy': 'Network firewall: Domain: Settings: Apply local connection security rules', + 'Policy': 'Network firewall: Domain: Settings: Apply ' + 'local connection security rules', 'lgpo_section': self.windows_firewall_gpedit_path, # Settings available are: # - Yes (default) From 1247598f28d0f0d117471df8e8ae93fa2384bbec Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 7 Dec 2018 17:18:29 -0700 Subject: [PATCH 26/55] Add more policies Adds more policies Fixes bug in netsh salt util Uses __context__ to speed things up Fixes bug in the state where it was reporting changes when no changes occurred --- salt/modules/win_lgpo.py | 664 ++++++++++++++++++++++++++++++++++- salt/states/win_lgpo.py | 16 +- salt/utils/win_lgpo_netsh.py | 2 +- 3 files changed, 666 insertions(+), 16 deletions(-) diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py index 45858c5c1023..674fd12d2196 100644 --- a/salt/modules/win_lgpo.py +++ b/salt/modules/win_lgpo.py @@ -488,6 +488,21 @@ def __init__(self): 'disable': 'No', 'notconfigured': 'Not configured' } + self.firewall_log_packets_connections = { + 'enable': 'Yes', + 'disable': 'No (default)', + 'notconfigured': 'Not configured' + } + self.firewall_notification = { + 'enable': 'Yes', + 'disable': 'No', + 'notconfigured': 'Not configured' + } + self.firewall_state = { + 'on': 'On (recommended)', + 'off': 'Off', + 'notconfigured': 'Not configured' + } self.krb_encryption_types = { 0: 'No minimum', 1: 'DES_CBC_CRC', @@ -869,6 +884,84 @@ def __init__(self): }, }, }, + 'WfwDomainState': { + 'Policy': 'Network firewall: Domain: State', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - On (recommended) + # - Off + # - Not configured + 'Settings': self.firewall_state.keys(), + 'NetSH': { + 'Profile': 'domain', + 'Section': 'state', + 'Option': 'State' # Unused, but needed + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_state, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_state, + 'value_lookup': True, + }, + }, + }, + 'WfwPrivateState': { + 'Policy': 'Network firewall: Private: State', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - On (recommended) + # - Off + # - Not configured + 'Settings': self.firewall_state.keys(), + 'NetSH': { + 'Profile': 'private', + 'Section': 'state', + 'Option': 'State' # Unused, but needed + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_state, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_state, + 'value_lookup': True, + }, + }, + }, + 'WfwPublicState': { + 'Policy': 'Network firewall: Public: State', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - On (recommended) + # - Off + # - Not configured + 'Settings': self.firewall_state.keys(), + 'NetSH': { + 'Profile': 'public', + 'Section': 'state', + 'Option': 'State' # Unused, but needed + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_state, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_state, + 'value_lookup': True, + }, + }, + }, 'WfwDomainInboundConnections': { 'Policy': 'Network firewall: Domain: Inbound connections', 'lgpo_section': self.windows_firewall_gpedit_path, @@ -896,6 +989,60 @@ def __init__(self): }, }, }, + 'WfwPrivateInboundConnections': { + 'Policy': 'Network firewall: Private: Inbound connections', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Block (default) + # - Block all connections + # - Allow + # - Not configured + 'Settings': self.firewall_inbound_connections.keys(), + 'NetSH': { + 'Profile': 'private', + 'Section': 'firewallpolicy', + 'Option': 'Inbound' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_inbound_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_inbound_connections, + 'value_lookup': True, + }, + }, + }, + 'WfwPublicInboundConnections': { + 'Policy': 'Network firewall: Public: Inbound connections', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Block (default) + # - Block all connections + # - Allow + # - Not configured + 'Settings': self.firewall_inbound_connections.keys(), + 'NetSH': { + 'Profile': 'public', + 'Section': 'firewallpolicy', + 'Option': 'Inbound' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_inbound_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_inbound_connections, + 'value_lookup': True, + }, + }, + }, 'WfwDomainOutboundConnections': { 'Policy': 'Network firewall: Domain: Outbound connections', 'lgpo_section': self.windows_firewall_gpedit_path, @@ -922,6 +1069,136 @@ def __init__(self): }, }, }, + 'WfwPrivateOutboundConnections': { + 'Policy': 'Network firewall: Private: Outbound connections', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Block + # - Allow (default) + # - Not configured + 'Settings': self.firewall_outbound_connections.keys(), + 'NetSH': { + 'Profile': 'private', + 'Section': 'firewallpolicy', + 'Option': 'Outbound' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_outbound_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_outbound_connections, + 'value_lookup': True, + }, + }, + }, + 'WfwPublicOutboundConnections': { + 'Policy': 'Network firewall: Public: Outbound connections', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Block + # - Allow (default) + # - Not configured + 'Settings': self.firewall_outbound_connections.keys(), + 'NetSH': { + 'Profile': 'public', + 'Section': 'firewallpolicy', + 'Option': 'Outbound' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_outbound_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_outbound_connections, + 'value_lookup': True, + }, + }, + }, + 'WfwDomainSettingsNotification': { + 'Policy': 'Network firewall: Domain: Settings: Display a notification', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes + # - No + # - Not configured + 'Settings': self.firewall_notification.keys(), + 'NetSH': { + 'Profile': 'domain', + 'Section': 'settings', + 'Option': 'InboundUserNotification' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_notification, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_notification, + 'value_lookup': True, + }, + }, + }, + 'WfwPrivateSettingsNotification': { + 'Policy': 'Network firewall: Private: Settings: Display a notification', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes + # - No + # - Not configured + 'Settings': self.firewall_notification.keys(), + 'NetSH': { + 'Profile': 'private', + 'Section': 'settings', + 'Option': 'InboundUserNotification' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_notification, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_notification, + 'value_lookup': True, + }, + }, + }, + 'WfwPublicSettingsNotification': { + 'Policy': 'Network firewall: Public: Settings: Display a notification', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes + # - No + # - Not configured + 'Settings': self.firewall_notification.keys(), + 'NetSH': { + 'Profile': 'public', + 'Section': 'settings', + 'Option': 'InboundUserNotification' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_notification, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_notification, + 'value_lookup': True, + }, + }, + }, 'WfwDomainSettingsLocalFirewallRules': { 'Policy': 'Network firewall: Domain: Settings: Apply ' 'local firewall rules', @@ -949,6 +1226,60 @@ def __init__(self): }, }, }, + 'WfwPrivateSettingsLocalFirewallRules': { + 'Policy': 'Network firewall: Private: Settings: Apply ' + 'local firewall rules', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes (default) + # - No + # - Not configured + 'Settings': self.firewall_rule_merging.keys(), + 'NetSH': { + 'Profile': 'private', + 'Section': 'settings', + 'Option': 'LocalFirewallRules' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': True, + }, + }, + }, + 'WfwPublicSettingsLocalFirewallRules': { + 'Policy': 'Network firewall: Public: Settings: Apply ' + 'local firewall rules', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes (default) + # - No + # - Not configured + 'Settings': self.firewall_rule_merging.keys(), + 'NetSH': { + 'Profile': 'public', + 'Section': 'settings', + 'Option': 'LocalFirewallRules' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': True, + }, + }, + }, 'WfwDomainSettingsLocalConnectionRules': { 'Policy': 'Network firewall: Domain: Settings: Apply ' 'local connection security rules', @@ -976,6 +1307,294 @@ def __init__(self): }, }, }, + 'WfwPrivateSettingsLocalConnectionRules': { + 'Policy': 'Network firewall: Private: Settings: Apply ' + 'local connection security rules', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes (default) + # - No + # - Not configured + 'Settings': self.firewall_rule_merging.keys(), + 'NetSH': { + 'Profile': 'private', + 'Section': 'settings', + 'Option': 'LocalConSecRules' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': True, + }, + }, + }, + 'WfwPublicSettingsLocalConnectionRules': { + 'Policy': 'Network firewall: Public: Settings: Apply ' + 'local connection security rules', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes (default) + # - No + # - Not configured + 'Settings': self.firewall_rule_merging.keys(), + 'NetSH': { + 'Profile': 'public', + 'Section': 'settings', + 'Option': 'LocalConSecRules' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_rule_merging, + 'value_lookup': True, + }, + }, + }, + 'WfwDomainLoggingName': { + 'Policy': 'Network firewall: Domain: Logging: Name', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - + # - Not configured + 'Settings': None, + 'NetSH': { + 'Profile': 'domain', + 'Section': 'logging', + 'Option': 'FileName' + } + }, + 'WfwPrivateLoggingName': { + 'Policy': 'Network firewall: Private: Logging: Name', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - + # - Not configured + 'Settings': None, + 'NetSH': { + 'Profile': 'private', + 'Section': 'logging', + 'Option': 'FileName' + } + }, + 'WfwPublicLoggingName': { + 'Policy': 'Network firewall: Public: Logging: Name', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - + # - Not configured + 'Settings': None, + 'NetSH': { + 'Profile': 'public', + 'Section': 'logging', + 'Option': 'FileName' + } + }, + 'WfwDomainLoggingMaxFileSize': { + 'Policy': 'Network firewall: Domain: Logging: Size limit (KB)', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - + # - Not configured + 'Settings': None, + 'NetSH': { + 'Profile': 'domain', + 'Section': 'logging', + 'Option': 'MaxFileSize' + } + }, + 'WfwPrivateLoggingMaxFileSize': { + 'Policy': 'Network firewall: Private: Logging: Size limit (KB)', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - + # - Not configured + 'Settings': None, + 'NetSH': { + 'Profile': 'private', + 'Section': 'logging', + 'Option': 'MaxFileSize' + } + }, + 'WfwPublicLoggingMaxFileSize': { + 'Policy': 'Network firewall: Public: Logging: Size limit (KB)', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - + # - Not configured + 'Settings': None, + 'NetSH': { + 'Profile': 'public', + 'Section': 'logging', + 'Option': 'MaxFileSize' + } + }, + 'WfwDomainLoggingAllowedConnections': { + 'Policy': 'Network firewall: Domain: Logging: Log successful connections', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes + # - No (default) + # - Not configured + 'Settings': self.firewall_log_packets_connections.keys(), + 'NetSH': { + 'Profile': 'domain', + 'Section': 'logging', + 'Option': 'LogAllowedConnections' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': True, + }, + }, + }, + 'WfwPrivateLoggingAllowedConnections': { + 'Policy': 'Network firewall: Private: Logging: Log successful connections', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes + # - No (default) + # - Not configured + 'Settings': self.firewall_log_packets_connections.keys(), + 'NetSH': { + 'Profile': 'private', + 'Section': 'logging', + 'Option': 'LogAllowedConnections' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': True, + }, + }, + }, + 'WfwPublicLoggingAllowedConnections': { + 'Policy': 'Network firewall: Public: Logging: Log successful connections', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes + # - No (default) + # - Not configured + 'Settings': self.firewall_log_packets_connections.keys(), + 'NetSH': { + 'Profile': 'public', + 'Section': 'logging', + 'Option': 'LogAllowedConnections' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': True, + }, + }, + }, + 'WfwDomainLoggingDroppedConnections': { + 'Policy': 'Network firewall: Domain: Logging: Log dropped packets', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes + # - No (default) + # - Not configured + 'Settings': self.firewall_log_packets_connections.keys(), + 'NetSH': { + 'Profile': 'domain', + 'Section': 'logging', + 'Option': 'LogDroppedConnections' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': True, + }, + }, + }, + 'WfwPrivateLoggingDroppedConnections': { + 'Policy': 'Network firewall: Private: Logging: Log dropped packets', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes + # - No (default) + # - Not configured + 'Settings': self.firewall_log_packets_connections.keys(), + 'NetSH': { + 'Profile': 'private', + 'Section': 'logging', + 'Option': 'LogDroppedConnections' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': True, + }, + }, + }, + 'WfwPublicLoggingDroppedConnections': { + 'Policy': 'Network firewall: Public: Logging: Log dropped packets', + 'lgpo_section': self.windows_firewall_gpedit_path, + # Settings available are: + # - Yes + # - No (default) + # - Not configured + 'Settings': self.firewall_log_packets_connections.keys(), + 'NetSH': { + 'Profile': 'public', + 'Section': 'logging', + 'Option': 'LogDroppedConnections' + }, + 'Transform': { + 'Get': '_dict_lookup', + 'Put': '_dict_lookup', + 'GetArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': False, + }, + 'PutArgs': { + 'lookup': self.firewall_log_packets_connections, + 'value_lookup': True, + }, + }, + }, 'PasswordHistory': { 'Policy': 'Enforce password history', 'lgpo_section': self.password_policy_gpedit_path, @@ -3731,24 +4350,51 @@ def _buildElementNsmap(using_elements): def _findOptionValueNetSH(profile, option): - settings = salt.utils.win_lgpo_netsh.get_all_settings(profile=profile, - store='lgpo') - return settings[option] + if 'lgpo.netsh_data' not in __context__: + __context__['lgpo.netsh_data'] = {} + + if profile not in __context__['lgpo.netsh_data']: + log.debug('LGPO: Loading netsh data for {0} profile'.format(profile)) + settings = salt.utils.win_lgpo_netsh.get_all_settings(profile=profile, + store='lgpo') + __context__['lgpo.netsh_data'].update({profile: settings}) + log.debug('LGPO: netsh returning value: {0}' + ''.format(__context__['lgpo.netsh_data'][profile][option])) + return __context__['lgpo.netsh_data'][profile][option] def _setOptionValueNetSH(profile, section, option, value): + if section not in ('firewallpolicy', 'settings', 'logging', 'state'): + raise ValueError('LGPO: Invalid section: {0}'.format(section)) + log.debug('LGPO: Setting the following\n' + 'Profile: {0}\n' + 'Section: {1}\n' + 'Option: {2}\n' + 'Value: {3}'.format(profile, section, option, value)) if section == 'firewallpolicy': - return salt.utils.win_lgpo_netsh.set_firewall_settings( + salt.utils.win_lgpo_netsh.set_firewall_settings( profile=profile, inbound=value if option == 'Inbound' else None, outbound=value if option == 'Outbound' else None, store='lgpo') if section == 'settings': - return salt.utils.win_lgpo_netsh.set_settings( - profile=profile, - setting=option, - value=value, - store='lgpo') + salt.utils.win_lgpo_netsh.set_settings( + profile=profile, setting=option, value=value, store='lgpo') + if section == 'state': + salt.utils.win_lgpo_netsh.set_state( + profile=profile, state=value, store='lgpo') + if section == 'logging': + if option in ('FileName', 'MaxFileSize'): + if value == 'Not configured': + value = 'notconfigured' + # Trim log for the two logging options + if option.startswith('Log'): + option = option[3:] + salt.utils.win_lgpo_netsh.set_logging_settings( + profile=profile, setting=option, value=value, store='lgpo') + log.debug('LGPO: Clearing netsh data for {0} profile'.format(profile)) + __context__['lgpo.netsh_data'].pop(profile) + return True def _findOptionValueInSeceditFile(option): diff --git a/salt/states/win_lgpo.py b/salt/states/win_lgpo.py index 188b3a57e646..ac80e07b9f20 100644 --- a/salt/states/win_lgpo.py +++ b/salt/states/win_lgpo.py @@ -308,13 +308,13 @@ def set_(name, policy_changes.append(policy_name) else: if additional_policy_comments: - ret['comment'] = '"{0}" is already set ({1}).\n'.format(policy_name, ', '.join(additional_policy_comments)) + ret['comment'] = '"{0}" is already set ({1})\n'.format(policy_name, ', '.join(additional_policy_comments)) else: - ret['comment'] = '"{0}" is already set.\n'.format(policy_name) + ret['comment'] + ret['comment'] = '"{0}" is already set\n'.format(policy_name) + ret['comment'] else: log.debug('%s current setting matches ' 'the requested setting', policy_name) - ret['comment'] = '"{0}" is already set.\n'.format(policy_name) + ret['comment'] + ret['comment'] = '"{0}" is already set\n'.format(policy_name) + ret['comment'] else: policy_changes.append(policy_name) log.debug('policy %s is not set, we will configure it', @@ -322,7 +322,7 @@ def set_(name, if __opts__['test']: if policy_changes: ret['result'] = None - ret['comment'] = 'The following policies are set to change:\n{0}.'.format( + ret['comment'] = 'The following policies are set to change:\n{0}'.format( '\n'.join(policy_changes)) else: ret['comment'] = 'All specified policies are properly configured' @@ -334,13 +334,17 @@ def set_(name, adml_language=adml_language) if _ret: ret['result'] = _ret - ret['comment'] = 'The following policies changed:\n{0}.'.format( - '\n'.join(policy_changes)) ret['changes'] = salt.utils.dictdiffer.deep_diff( current_policy, __salt__['lgpo.get'](policy_class=policy_class, adml_language=adml_language, hierarchical_return=False)) + if ret['changes']: + ret['comment'] = 'The following policies changed:\n{0}' \ + ''.format('\n'.join(policy_changes)) + else: + ret['comment'] = 'The following policies are in the correct state:\n{0}' \ + ''.format('\n'.join(policy_changes)) else: ret['result'] = False ret['comment'] = 'Errors occurred while attempting to configure policies: {0}'.format(_ret) diff --git a/salt/utils/win_lgpo_netsh.py b/salt/utils/win_lgpo_netsh.py index c548b8d24ca9..fd19710a1f9c 100644 --- a/salt/utils/win_lgpo_netsh.py +++ b/salt/utils/win_lgpo_netsh.py @@ -443,7 +443,7 @@ def set_logging_settings(profile, setting, value, store='local'): int(value) except ValueError: raise ValueError('Incorrect value: {0}'.format(value)) - if not 1 >= int(value) <= 32767: + if not 1 <= int(value) <= 32767: raise ValueError('Incorrect value: {0}'.format(value)) # Run the command command = 'set {0}profile logging {1} {2}'.format(profile, setting, value) From c55b7b4343a35e8f132d5fdbc061a41590b9da92 Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 7 Dec 2018 17:30:00 -0700 Subject: [PATCH 27/55] Fix some lint --- salt/modules/win_firewall.py | 2 +- salt/modules/win_lgpo.py | 2 +- salt/utils/win_lgpo_netsh.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/salt/modules/win_firewall.py b/salt/modules/win_firewall.py index 5ebd5207168c..f232b8690811 100644 --- a/salt/modules/win_firewall.py +++ b/salt/modules/win_firewall.py @@ -559,7 +559,7 @@ def set_firewall_settings(profile, inbound=None, outbound=None, store='local'): def set_logging_settings(profile, setting, value, store='local'): - ''' + r''' Configure logging settings for the Windows firewall. .. versionadded:: 2018.3.4 diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py index 674fd12d2196..3cd389c98c80 100644 --- a/salt/modules/win_lgpo.py +++ b/salt/modules/win_lgpo.py @@ -137,7 +137,7 @@ class _policy_info(object): - ''' + r''' Policy Helper Class =================== diff --git a/salt/utils/win_lgpo_netsh.py b/salt/utils/win_lgpo_netsh.py index fd19710a1f9c..d7fb46416c8c 100644 --- a/salt/utils/win_lgpo_netsh.py +++ b/salt/utils/win_lgpo_netsh.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -''' +r''' A salt util for modifying firewall settings. .. versionadded:: 2018.3.4 @@ -195,7 +195,7 @@ def get_settings(profile, section, store='local'): ret = {} # Skip the first 2 lines. Add everything else to a dictionary for line in results[3:]: - ret.update(dict(map(None, *[iter(re.split(r"\s{2,}", line))]*2))) + ret.update(dict(map(None, *[iter(re.split(r"\s{2,}", line))]*2))) # pylint: disable=incompatible-py3-code # Remove spaces from the values so that `Not Configured` is detected # correctly From 2dde12add35568d394642db8ad747af66b0dd362 Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 7 Dec 2018 17:42:37 -0700 Subject: [PATCH 28/55] Fix one more lint item --- salt/utils/win_lgpo_netsh.py | 1 + 1 file changed, 1 insertion(+) diff --git a/salt/utils/win_lgpo_netsh.py b/salt/utils/win_lgpo_netsh.py index d7fb46416c8c..3427144b6bca 100644 --- a/salt/utils/win_lgpo_netsh.py +++ b/salt/utils/win_lgpo_netsh.py @@ -77,6 +77,7 @@ import salt.modules.cmdmod from salt.exceptions import CommandExecutionError +from salt.ext.six.moves import map log = logging.getLogger(__name__) __hostname__ = socket.gethostname() From f077783cbde0e07174e8163a4d491c2f852ed4c8 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Mon, 10 Dec 2018 13:40:53 -0700 Subject: [PATCH 29/55] Add some tests --- salt/modules/win_firewall.py | 3 +- tests/unit/utils/test_win_lgpo_netsh.py | 535 ++++++++++++++++++++++++ 2 files changed, 537 insertions(+), 1 deletion(-) create mode 100644 tests/unit/utils/test_win_lgpo_netsh.py diff --git a/salt/modules/win_firewall.py b/salt/modules/win_firewall.py index f232b8690811..4ad2ce086920 100644 --- a/salt/modules/win_firewall.py +++ b/salt/modules/win_firewall.py @@ -443,7 +443,8 @@ def get_all_settings(domain, store='local'): # defined by local group policy salt * win_firewall.get_all_settings domain lgpo ''' - return salt.utils.win_lgpo_netsh.get_all_settings(domain, store) + return salt.utils.win_lgpo_netsh.get_all_settings(domain=domain, + store=store) def get_all_profiles(store='local'): diff --git a/tests/unit/utils/test_win_lgpo_netsh.py b/tests/unit/utils/test_win_lgpo_netsh.py new file mode 100644 index 000000000000..b7098b0e0dc3 --- /dev/null +++ b/tests/unit/utils/test_win_lgpo_netsh.py @@ -0,0 +1,535 @@ +# -*- coding: utf-8 -*- + +# Import Python Libs +from __future__ import absolute_import, unicode_literals, print_function + +# Import Salt Testing Libs +from tests.support.helpers import destructiveTest +from tests.support.mock import NO_MOCK, NO_MOCK_REASON +from tests.support.unit import TestCase, skipIf + +# Import Salt Libs +import salt.utils.platform +import salt.utils.win_lgpo_netsh as win_lgpo_netsh +from salt.exceptions import CommandExecutionError + + +@skipIf(NO_MOCK, NO_MOCK_REASON) +@skipIf(not salt.utils.platform.is_windows(), 'System is not Windows') +class WinLgpoNetshTestCase(TestCase): + def test_get_settings_firewallpolicy_local(self): + ret = win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy', + store='local') + self.assertIn('Inbound', ret) + self.assertIn('Outbound', ret) + + def test_get_settings_firewallpolicy_lgpo(self): + ret = win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy', + store='lgpo') + self.assertIn('Inbound', ret) + self.assertIn('Outbound', ret) + + def test_get_settings_logging_local(self): + ret = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='local') + self.assertIn('FileName', ret) + self.assertIn('LogAllowedConnections', ret) + self.assertIn('LogDroppedConnections', ret) + self.assertIn('MaxFileSize', ret) + + def test_get_settings_logging_lgpo(self): + ret = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='lgpo') + self.assertIn('FileName', ret) + self.assertIn('LogAllowedConnections', ret) + self.assertIn('LogDroppedConnections', ret) + self.assertIn('MaxFileSize', ret) + + def test_get_settings_settings_local(self): + ret = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='local') + self.assertIn('InboundUserNotification', ret) + self.assertIn('LocalConSecRules', ret) + self.assertIn('LocalFirewallRules', ret) + self.assertIn('RemoteManagement', ret) + self.assertIn('UnicastResponseToMulticast', ret) + + def test_get_settings_settings_lgpo(self): + ret = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='lgpo') + self.assertIn('InboundUserNotification', ret) + self.assertIn('LocalConSecRules', ret) + self.assertIn('LocalFirewallRules', ret) + self.assertIn('RemoteManagement', ret) + self.assertIn('UnicastResponseToMulticast', ret) + + def test_get_settings_state_local(self): + ret = win_lgpo_netsh.get_settings(profile='domain', + section='state', + store='local') + self.assertIn('State', ret) + + def test_get_settings_state_lgpo(self): + ret = win_lgpo_netsh.get_settings(profile='domain', + section='state', + store='lgpo') + self.assertIn('State', ret) + + def test_get_all_settings_local(self): + ret = win_lgpo_netsh.get_all_settings(profile='domain', + store='local') + + self.assertIn('Inbound', ret) + self.assertIn('Outbound', ret) + self.assertIn('FileName', ret) + self.assertIn('LogAllowedConnections', ret) + self.assertIn('LogDroppedConnections', ret) + self.assertIn('MaxFileSize', ret) + self.assertIn('InboundUserNotification', ret) + self.assertIn('LocalConSecRules', ret) + self.assertIn('LocalFirewallRules', ret) + self.assertIn('RemoteManagement', ret) + self.assertIn('UnicastResponseToMulticast', ret) + self.assertIn('State', ret) + + def test_get_all_settings_lgpo(self): + ret = win_lgpo_netsh.get_all_settings(profile='domain', + store='local') + + self.assertIn('Inbound', ret) + self.assertIn('Outbound', ret) + self.assertIn('FileName', ret) + self.assertIn('LogAllowedConnections', ret) + self.assertIn('LogDroppedConnections', ret) + self.assertIn('MaxFileSize', ret) + self.assertIn('InboundUserNotification', ret) + self.assertIn('LocalConSecRules', ret) + self.assertIn('LocalFirewallRules', ret) + self.assertIn('RemoteManagement', ret) + self.assertIn('UnicastResponseToMulticast', ret) + self.assertIn('State', ret) + + def test_get_all_profiles_local(self): + ret = win_lgpo_netsh.get_all_profiles(store='local') + self.assertIn('Domain Profile', ret) + self.assertIn('Private Profile', ret) + self.assertIn('Public Profile', ret) + + def test_get_all_profiles_lgpo(self): + ret = win_lgpo_netsh.get_all_profiles(store='lgpo') + self.assertIn('Domain Profile', ret) + self.assertIn('Private Profile', ret) + self.assertIn('Public Profile', ret) + + @destructiveTest + def test_set_firewall_settings_inbound_local(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy', + store='local')['Inbound'] + try: + ret = win_lgpo_netsh.set_firewall_settings(profile='domain', + inbound='allowinbound', + store='local') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy', + store='local')['Inbound'] + self.assertEqual('AllowInbound', new) + finally: + ret = win_lgpo_netsh.set_firewall_settings(profile='domain', + inbound=current, + store='local') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_settings_inbound_local_notconfigured(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy', + store='local')['Inbound'] + try: + self.assertRaises( + CommandExecutionError, + win_lgpo_netsh.set_firewall_settings, + profile='domain', + inbound='notconfigured', + store='local') + finally: + ret = win_lgpo_netsh.set_firewall_settings(profile='domain', + inbound=current, + store='local') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_settings_inbound_lgpo_notconfigured(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy', + store='lgpo')['Inbound'] + try: + ret = win_lgpo_netsh.set_firewall_settings(profile='domain', + inbound='notconfigured', + store='lgpo') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy', + store='lgpo')['Inbound'] + self.assertEqual('NotConfigured', new) + finally: + ret = win_lgpo_netsh.set_firewall_settings(profile='domain', + inbound=current, + store='lgpo') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_settings_outbound_local(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy', + store='local')['Outbound'] + try: + ret = win_lgpo_netsh.set_firewall_settings(profile='domain', + outbound='allowoutbound', + store='local') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='firewallpolicy', + store='local')['Outbound'] + self.assertEqual('AllowOutbound', new) + finally: + ret = win_lgpo_netsh.set_firewall_settings(profile='domain', + outbound=current, + store='local') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_logging_allowed_local_enable(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='local')['LogAllowedConnections'] + try: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='allowedconnections', + value='enable', + store='local') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='local')['LogAllowedConnections'] + self.assertEqual('Enable', new) + finally: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='allowedconnections', + value=current, + store='local') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_logging_allowed_local_notconfigured(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='local')['LogAllowedConnections'] + try: + self.assertRaises( + CommandExecutionError, + win_lgpo_netsh.set_logging_settings, + profile='domain', + setting='allowedconnections', + value='notconfigured', + store='local') + finally: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='allowedconnections', + value=current, + store='local') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_logging_allowed_lgpo_notconfigured(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='lgpo')['LogAllowedConnections'] + try: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='allowedconnections', + value='notconfigured', + store='lgpo') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='lgpo')['LogAllowedConnections'] + self.assertEqual('NotConfigured', new) + finally: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='allowedconnections', + value=current, + store='lgpo') + self.assertTrue(ret) + + def test_set_firewall_logging_dropped_local_enable(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='local')['LogDroppedConnections'] + try: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='droppedconnections', + value='enable', + store='local') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='local')['LogDroppedConnections'] + self.assertEqual('Enable', new) + finally: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='droppedconnections', + value=current, + store='local') + self.assertTrue(ret) + + def test_set_firewall_logging_filename_local(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='local')['FileName'] + try: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='filename', + value='C:\\Temp\\test.log', + store='local') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='local')['FileName'] + self.assertEqual('C:\\Temp\\test.log', new) + finally: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='filename', + value=current, + store='local') + self.assertTrue(ret) + + def test_set_firewall_logging_maxfilesize_local(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='local')['MaxFileSize'] + try: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='maxfilesize', + value='16384', + store='local') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='logging', + store='local')['MaxFileSize'] + self.assertEqual('16384', new) + finally: + ret = win_lgpo_netsh.set_logging_settings(profile='domain', + setting='maxfilesize', + value=current, + store='local') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_settings_fwrules_local_enable(self): + self.assertRaises( + win_lgpo_netsh.set_settings, + profile='domain', + setting='localfirewallrules', + value='enable', + store='local') + + @destructiveTest + def test_set_firewall_settings_fwrules_lgpo_notconfigured(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='lgpo')['LocalFirewallRules'] + try: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='localfirewallrules', + value='notconfigured', + store='lgpo') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='lgpo')['LocalFirewallRules'] + self.assertEqual('NotConfigured', new) + finally: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='localfirewallrules', + value=current, + store='lgpo') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_settings_consecrules_local_enable(self): + self.assertRaises( + win_lgpo_netsh.set_settings, + profile='domain', + setting='localconsecrules', + value='enable', + store='local') + + def test_set_firewall_settings_notification_local_enable(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='local')['InboundUserNotification'] + try: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='inboundusernotification', + value='enable', + store='local') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='local')['InboundUserNotification'] + self.assertEqual('Enable', new) + finally: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='inboundusernotification', + value=current, + store='local') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_settings_notification_local_notconfigured(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='local')['InboundUserNotification'] + try: + self.assertRaises( + CommandExecutionError, + win_lgpo_netsh.set_settings, + profile='domain', + setting='inboundusernotification', + value='notconfigured', + store='local') + finally: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='inboundusernotification', + value=current, + store='local') + self.assertTrue(ret) + + def test_set_firewall_settings_notification_lgpo_notconfigured(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='lgpo')['InboundUserNotification'] + try: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='inboundusernotification', + value='notconfigured', + store='lgpo') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='lgpo')['InboundUserNotification'] + self.assertEqual('NotConfigured', new) + finally: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='inboundusernotification', + value=current, + store='lgpo') + self.assertTrue(ret) + + def test_set_firewall_settings_remotemgmt_local_enable(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='local')['RemoteManagement'] + try: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='remotemanagement', + value='enable', + store='local') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='local')['RemoteManagement'] + self.assertEqual('Enable', new) + finally: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='remotemanagement', + value=current, + store='local') + self.assertTrue(ret) + + def test_set_firewall_settings_unicast_local_disable(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='local')['UnicastResponseToMulticast'] + try: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='unicastresponsetomulticast', + value='disable', + store='local') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='settings', + store='local')['UnicastResponseToMulticast'] + self.assertEqual('Disable', new) + finally: + ret = win_lgpo_netsh.set_settings(profile='domain', + setting='unicastresponsetomulticast', + value=current, + store='local') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_state_local_on(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='state', + store='local')['State'] + try: + ret = win_lgpo_netsh.set_state(profile='domain', + state='off', + store='local') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='state', + store='local')['State'] + self.assertEqual('OFF', new) + finally: + ret = win_lgpo_netsh.set_state(profile='domain', + state=current, + store='local') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_state_local_notconfigured(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='state', + store='local')['State'] + try: + self.assertRaises( + CommandExecutionError, + win_lgpo_netsh.set_state, + profile='domain', + state='notconfigured', + store='local') + finally: + ret = win_lgpo_netsh.set_state(profile='domain', + state=current, + store='local') + self.assertTrue(ret) + + @destructiveTest + def test_set_firewall_state_lgpo_notconfigured(self): + current = win_lgpo_netsh.get_settings(profile='domain', + section='state', + store='local')['State'] + try: + ret = win_lgpo_netsh.set_state(profile='domain', + state='notconfigured', + store='lgpo') + self.assertTrue(ret) + new = win_lgpo_netsh.get_settings(profile='domain', + section='state', + store='lgpo')['State'] + self.assertEqual('NotConfigured', new) + finally: + ret = win_lgpo_netsh.set_state(profile='domain', + state=current, + store='lgpo') + self.assertTrue(ret) From 5eea728879cf6e545ce7db4254edd802ebaf0954 Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 10 Dec 2018 14:05:48 -0700 Subject: [PATCH 30/55] Fix some lint --- salt/modules/win_firewall.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/win_firewall.py b/salt/modules/win_firewall.py index 4ad2ce086920..fb6352888590 100644 --- a/salt/modules/win_firewall.py +++ b/salt/modules/win_firewall.py @@ -443,7 +443,7 @@ def get_all_settings(domain, store='local'): # defined by local group policy salt * win_firewall.get_all_settings domain lgpo ''' - return salt.utils.win_lgpo_netsh.get_all_settings(domain=domain, + return salt.utils.win_lgpo_netsh.get_all_settings(profile=domain, store=store) From 48fc01fb137f5df3e33d9b495d83a4d51db90020 Mon Sep 17 00:00:00 2001 From: Brett Benassi Date: Tue, 11 Dec 2018 10:24:44 -0700 Subject: [PATCH 31/55] Adding git 'v' to the bootstrap options --- .kitchen.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.kitchen.yml b/.kitchen.yml index 9483e7b27838..386e2b141ab3 100644 --- a/.kitchen.yml +++ b/.kitchen.yml @@ -30,7 +30,7 @@ provisioner: salt_install: bootstrap salt_version: latest salt_bootstrap_url: https://bootstrap.saltstack.com - salt_bootstrap_options: -X -p rsync git <%= version %> + salt_bootstrap_options: -X -p rsync git v<%= version %> log_level: info sudo: true require_chef: false From ec8e116f7e368a28790fc20dfe03eda7fc28e4e6 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Tue, 11 Dec 2018 13:58:15 -0600 Subject: [PATCH 32/55] Decode fetch results before making string comparisons --- salt/modules/mysql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/mysql.py b/salt/modules/mysql.py index 5d98c7d19774..9aac53640325 100644 --- a/salt/modules/mysql.py +++ b/salt/modules/mysql.py @@ -1778,7 +1778,7 @@ def user_grants(user, return False ret = [] - results = cur.fetchall() + results = salt.utils.data.decode(cur.fetchall()) for grant in results: tmp = grant[0].split(' IDENTIFIED BY')[0] if 'WITH GRANT OPTION' in grant[0] and 'WITH GRANT OPTION' not in tmp: From 56c0e55581cece689663ed0e49cdc00d5e02b183 Mon Sep 17 00:00:00 2001 From: twangboy Date: Tue, 11 Dec 2018 16:49:39 -0700 Subject: [PATCH 33/55] Fix issues with lgpo state and util Fix py3 issues in win_lgpo_netsh Make the state case insensitive for policy lookups --- salt/states/win_lgpo.py | 4 ++++ salt/utils/win_lgpo_netsh.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/salt/states/win_lgpo.py b/salt/states/win_lgpo.py index ac80e07b9f20..bc5b6b9b9b20 100644 --- a/salt/states/win_lgpo.py +++ b/salt/states/win_lgpo.py @@ -114,6 +114,7 @@ # Import 3rd party libs from salt.ext import six +from requests.structures import CaseInsensitiveDict log = logging.getLogger(__name__) __virtualname__ = 'lgpo' @@ -250,6 +251,9 @@ def set_(name, for policy_section, policy_data in six.iteritems(pol_data): pol_id = None if policy_data and policy_data['output_section'] in current_policy: + # Make the subdict keys case insensitive + current_policy[policy_data['output_section']] = CaseInsensitiveDict( + current_policy[policy_data['output_section']]) for policy_name, policy_setting in six.iteritems(policy_data['requested_policy']): currently_set = False if policy_name in current_policy[policy_data['output_section']]: diff --git a/salt/utils/win_lgpo_netsh.py b/salt/utils/win_lgpo_netsh.py index 3427144b6bca..c6e00d21e735 100644 --- a/salt/utils/win_lgpo_netsh.py +++ b/salt/utils/win_lgpo_netsh.py @@ -196,7 +196,7 @@ def get_settings(profile, section, store='local'): ret = {} # Skip the first 2 lines. Add everything else to a dictionary for line in results[3:]: - ret.update(dict(map(None, *[iter(re.split(r"\s{2,}", line))]*2))) # pylint: disable=incompatible-py3-code + ret.update(dict(zip(*[iter(re.split(r"\s{2,}", line))]*2))) # Remove spaces from the values so that `Not Configured` is detected # correctly From b855fd9e7c5711322497204bb231a95f99f95f9e Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Wed, 12 Dec 2018 09:55:08 -0600 Subject: [PATCH 34/55] Fix misspelling in comment --- salt/fileserver/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/fileserver/__init__.py b/salt/fileserver/__init__.py index 8f91366cbf9c..38b109c378a8 100644 --- a/salt/fileserver/__init__.py +++ b/salt/fileserver/__init__.py @@ -373,7 +373,7 @@ def backends(self, back=None): # Avoid error logging when performing lookups in the LazyDict by # instead doing the membership check on the result of a call to its - # .keys() attribute rather than on the LaztDict itself. + # .keys() attribute rather than on the LazyDict itself. server_funcs = self.servers.keys() try: subtract_only = all((x.startswith('-') for x in back)) From 1233e2b4807b890dd027223429252c4cdd932485 Mon Sep 17 00:00:00 2001 From: twangboy Date: Wed, 12 Dec 2018 09:10:43 -0700 Subject: [PATCH 35/55] Fix some lint --- salt/utils/win_lgpo_netsh.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/utils/win_lgpo_netsh.py b/salt/utils/win_lgpo_netsh.py index c6e00d21e735..a31cc7f33f8a 100644 --- a/salt/utils/win_lgpo_netsh.py +++ b/salt/utils/win_lgpo_netsh.py @@ -77,7 +77,7 @@ import salt.modules.cmdmod from salt.exceptions import CommandExecutionError -from salt.ext.six.moves import map +from salt.ext.six.moves import zip log = logging.getLogger(__name__) __hostname__ = socket.gethostname() @@ -196,7 +196,7 @@ def get_settings(profile, section, store='local'): ret = {} # Skip the first 2 lines. Add everything else to a dictionary for line in results[3:]: - ret.update(dict(zip(*[iter(re.split(r"\s{2,}", line))]*2))) + ret.update(dict(list(zip(*[iter(re.split(r"\s{2,}", line))]*2)))) # Remove spaces from the values so that `Not Configured` is detected # correctly From 9eaa2edfe390d2a7a7172384a2faf6ae8d6d1040 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Wed, 12 Dec 2018 11:11:12 -0600 Subject: [PATCH 36/55] Remove extraneous comments --- tests/unit/modules/test_ps.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/unit/modules/test_ps.py b/tests/unit/modules/test_ps.py index babc20ea91cd..dc1b0cfe8330 100644 --- a/tests/unit/modules/test_ps.py +++ b/tests/unit/modules/test_ps.py @@ -142,17 +142,6 @@ def test_disk_partition_usage(self): 'fstype': 'hfs'}, ps.disk_partitions()[0]) - ## Should only be tested in integration - # def test_total_physical_memory(self): - # pass - - ## Should only be tested in integration - # def test_num_cpus(self): - # pass - - ## Should only be tested in integration - # def test_boot_time(self): - # pass def test_network_io_counters(self): with patch('salt.utils.psutil_compat.net_io_counters', MagicMock(return_value=STUB_NETWORK_IO)): From 1a00abc5f63a987e9d9b707e3cf4e4ea9dbf3bc1 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Wed, 12 Dec 2018 11:41:18 -0600 Subject: [PATCH 37/55] Add unit test for _get_proc_cmdline --- tests/unit/modules/test_ps.py | 50 +++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/unit/modules/test_ps.py b/tests/unit/modules/test_ps.py index dc1b0cfe8330..139634cb3bea 100644 --- a/tests/unit/modules/test_ps.py +++ b/tests/unit/modules/test_ps.py @@ -6,12 +6,14 @@ # Import Python libs from __future__ import absolute_import, unicode_literals, print_function from collections import namedtuple +import time # Import Salt Testing libs from tests.support.unit import TestCase, skipIf from tests.support.mock import MagicMock, patch, call, Mock # Import Salt libs +import salt.utils.data import salt.modules.ps as ps HAS_PSUTIL_VERSION = False @@ -63,6 +65,48 @@ def _get_proc_pid(proc): return proc.pid +class DummyProcess(object): + ''' + Dummy class to emulate psutil.Process. This ensures that _any_ string + values used for any of the options passed in are converted to str types on + both Python 2 and Python 3. + ''' + def __init__(self, cmdline=None, create_time=None, name=None, status=None, + username=None, pid=None): + self._cmdline = salt.utils.data.decode( + cmdline if cmdline is not None else [], + to_str=True) + self._create_time = salt.utils.data.decode( + create_time if create_time is not None else time.time(), + to_str=True) + self._name = salt.utils.data.decode( + name if name is not None else [], + to_str=True) + self._status = salt.utils.data.decode(status, to_str=True) + self._username = salt.utils.data.decode(username, to_str=True) + self._pid = salt.utils.data.decode( + pid if pid is not None else 12345, + to_str=True) + + def cmdline(self): + return self._cmdline + + def create_time(self): + return self._create_time + + def name(self): + return self._name + + def status(self): + return self._status + + def username(self): + return self._username + + def pid(self): + return self._pid + + class PsTestCase(TestCase): def setUp(self): self.mocked_proc = mocked_proc = MagicMock('salt.utils.psutil_compat.Process') @@ -73,6 +117,12 @@ def setUp(self): self.mocked_proc.name = 'test_mock_proc' self.mocked_proc.pid = 9999999999 + @skipIf(not ps.PSUTIL2, 'Only run for psutil 2.x') + def test__get_proc_cmdline(self): + cmdline = ['echo', 'питон'] + ret = ps._get_proc_cmdline(DummyProcess(cmdline=cmdline)) + assert ret == cmdline, ret + def test_get_pid_list(self): with patch('salt.utils.psutil_compat.pids', MagicMock(return_value=STUB_PID_LIST)): From 3a3d9b7d3ee6fe0bf5e9761e26a6347ae6b81563 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Wed, 12 Dec 2018 11:41:43 -0600 Subject: [PATCH 38/55] Ensure that unicode types returned from compatibility funcs --- salt/modules/ps.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/salt/modules/ps.py b/salt/modules/ps.py index aef10d0682ba..bb37873f48c9 100644 --- a/salt/modules/ps.py +++ b/salt/modules/ps.py @@ -14,6 +14,7 @@ import re # Import salt libs +import salt.utils.data from salt.exceptions import SaltInvocationError, CommandExecutionError # Import third party libs @@ -53,9 +54,9 @@ def _get_proc_cmdline(proc): It's backward compatible with < 2.0 versions of psutil. ''' try: - return proc.cmdline() if PSUTIL2 else proc.cmdline + return salt.utils.data.decode(proc.cmdline() if PSUTIL2 else proc.cmdline) except (psutil.NoSuchProcess, psutil.AccessDenied): - return '' + return [] def _get_proc_create_time(proc): @@ -65,7 +66,7 @@ def _get_proc_create_time(proc): It's backward compatible with < 2.0 versions of psutil. ''' try: - return proc.create_time() if PSUTIL2 else proc.create_time + return salt.utils.data.decode(proc.create_time() if PSUTIL2 else proc.create_time) except (psutil.NoSuchProcess, psutil.AccessDenied): return None @@ -77,7 +78,7 @@ def _get_proc_name(proc): It's backward compatible with < 2.0 versions of psutil. ''' try: - return proc.name() if PSUTIL2 else proc.name + return salt.utils.data.decode(proc.name() if PSUTIL2 else proc.name) except (psutil.NoSuchProcess, psutil.AccessDenied): return [] @@ -89,7 +90,7 @@ def _get_proc_status(proc): It's backward compatible with < 2.0 versions of psutil. ''' try: - return proc.status() if PSUTIL2 else proc.status + return salt.utils.data.decode(proc.status() if PSUTIL2 else proc.status) except (psutil.NoSuchProcess, psutil.AccessDenied): return None @@ -101,7 +102,7 @@ def _get_proc_username(proc): It's backward compatible with < 2.0 versions of psutil. ''' try: - return proc.username() if PSUTIL2 else proc.username + return salt.utils.data.decode(proc.username() if PSUTIL2 else proc.username) except (psutil.NoSuchProcess, psutil.AccessDenied, KeyError): return None From 0d00ae4f8eb65f3cd3bef097760ff6f7a9227ab4 Mon Sep 17 00:00:00 2001 From: Benjamin Drung Date: Wed, 12 Dec 2018 19:58:00 +0100 Subject: [PATCH 39/55] Fix RemoveCapacityFromDiskgroupTestCase require pyvmomi If the `pyvmomi` library is missing, all tests from the tests.unit.modules.RemoveCapacityFromDiskgroupTestCase will fail: ``` ERROR: test__get_proxy_target_call (unit.modules.test_vsphere.RemoveCapacityFromDiskgroupTestCase) [CPU:0.0%|MEM:35.1%] ---------------------------------------------------------------------- Traceback (most recent call last): File "salt/utils/vmware.py", line 485, in disconnect Disconnect(service_instance) NameError: name 'Disconnect' is not defined During handling of the above exception, another exception occurred: Traceback (most recent call last): File "salt/modules/vsphere.py", line 381, in _gets_service_instance_via_proxy salt.utils.vmware.disconnect(local_service_instance) File "salt/utils/vmware.py", line 486, in disconnect except vim.fault.NoPermission as exc: NameError: name 'vim' is not defined During handling of the above exception, another exception occurred: Traceback (most recent call last): File "salt/utils/vmware.py", line 485, in disconnect Disconnect(service_instance) NameError: name 'Disconnect' is not defined During handling of the above exception, another exception occurred: Traceback (most recent call last): File "tests/unit/modules/test_vsphere.py", line 1568, in test__get_proxy_target_call capacity_disk_ids=['fake_disk1', 'fake_disk2']) File "salt/modules/vsphere.py", line 295, in __supports_proxies return fn(*args, **salt.utils.args.clean_kwargs(**kwargs)) File "salt/modules/vsphere.py", line 386, in _gets_service_instance_via_proxy salt.utils.vmware.disconnect(local_service_instance) File "/home/bdrung/projects/salt/debian/salt/utils/vmware.py", line 486, in disconnect except vim.fault.NoPermission as exc: NameError: name 'vim' is not defined ``` Signed-off-by: Benjamin Drung --- tests/unit/modules/test_vsphere.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/modules/test_vsphere.py b/tests/unit/modules/test_vsphere.py index 916c09d2c6b3..5b61aa0257d2 100644 --- a/tests/unit/modules/test_vsphere.py +++ b/tests/unit/modules/test_vsphere.py @@ -1478,6 +1478,7 @@ def test_success_output(self): @skipIf(NO_MOCK, NO_MOCK_REASON) +@skipIf(not HAS_PYVMOMI, 'The \'pyvmomi\' library is missing') @skipIf(not vsphere.HAS_JSONSCHEMA, 'The \'jsonschema\' library is missing') class RemoveCapacityFromDiskgroupTestCase(TestCase, LoaderModuleMockMixin): ''' From ea90f114a2e3d3381fe832af52eaaaaaee2805a1 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 13 Dec 2018 12:29:45 -0700 Subject: [PATCH 40/55] Let minions try to connect to master indefinitly --- salt/minion.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/salt/minion.py b/salt/minion.py index 0a1bc3ace4f0..22ece7c9de62 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -976,9 +976,8 @@ def _check_minions(self): ''' if not self.minions: err = ('Minion unable to successfully connect to ' - 'a Salt Master. Exiting.') + 'a Salt Master.') log.error(err) - raise SaltSystemExit(code=42, msg=err) def _spawn_minions(self, timeout=60): ''' From ebc9a01b38918e9bb8c1dee8969392db51734b08 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Thu, 13 Dec 2018 13:08:40 -0800 Subject: [PATCH 41/55] When reading and writing the key cache file, when using Python3, ensuring the file is read & written in binary mode. --- salt/master.py | 8 ++++++-- salt/utils/minions.py | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/salt/master.py b/salt/master.py index f03098d675b9..30983c306862 100644 --- a/salt/master.py +++ b/salt/master.py @@ -250,8 +250,12 @@ def handle_key_cache(self): keys.append(fn_) log.debug('Writing master key cache') # Write a temporary file securely - with salt.utils.atomicfile.atomic_open(os.path.join(self.opts['pki_dir'], acc, '.key_cache')) as cache_file: - self.serial.dump(keys, cache_file) + if six.PY2: + with salt.utils.atomicfile.atomic_open(os.path.join(self.opts['pki_dir'], acc, '.key_cache')) as cache_file: + self.serial.dump(keys, cache_file) + else: + with salt.utils.atomicfile.atomic_open(os.path.join(self.opts['pki_dir'], acc, '.key_cache'), mode='wb') as cache_file: + self.serial.dump(keys, cache_file) def handle_key_rotate(self, now): ''' diff --git a/salt/utils/minions.py b/salt/utils/minions.py index 8056da273158..e327dedf0e56 100644 --- a/salt/utils/minions.py +++ b/salt/utils/minions.py @@ -245,8 +245,12 @@ def _pki_minions(self): try: if self.opts['key_cache'] and os.path.exists(pki_cache_fn): log.debug('Returning cached minion list') - with salt.utils.files.fopen(pki_cache_fn) as fn_: - return self.serial.load(fn_) + if six.PY2: + with salt.utils.files.fopen(pki_cache_fn) as fn_: + return self.serial.load(fn_) + else: + with salt.utils.files.fopen(pki_cache_fn, mode='rb') as fn_: + return self.serial.load(fn_) else: for fn_ in salt.utils.data.sorted_ignorecase(os.listdir(os.path.join(self.opts['pki_dir'], self.acc))): if not fn_.startswith('.') and os.path.isfile(os.path.join(self.opts['pki_dir'], self.acc, fn_)): From 10cb130eb6c6cf9c97636479dade099463131c86 Mon Sep 17 00:00:00 2001 From: Ch3LL Date: Fri, 14 Dec 2018 13:18:11 -0500 Subject: [PATCH 42/55] Add tempfile import in file tests --- tests/integration/states/test_file.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index eaddef19ff3f..516b71cc6bf8 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -13,6 +13,7 @@ import sys import shutil import stat +import tempfile import textwrap import filecmp From 8997550040301f08af21045b4a55c763457f5044 Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 14 Dec 2018 14:12:49 -0700 Subject: [PATCH 43/55] Fix unit tests --- tests/unit/modules/test_win_groupadd.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/unit/modules/test_win_groupadd.py b/tests/unit/modules/test_win_groupadd.py index 93a6b3f5466a..048112d893f0 100644 --- a/tests/unit/modules/test_win_groupadd.py +++ b/tests/unit/modules/test_win_groupadd.py @@ -97,7 +97,7 @@ def test_add_group_exists(self): 'members': ['HOST\\spongebob']}) with patch.object(win_groupadd, 'info', info),\ patch.object(win_groupadd, '_get_computer_object', Mock()): - self.assertTrue(win_groupadd.add('foo')) + self.assertFalse(win_groupadd.add('foo')) def test_add_error(self): ''' @@ -131,7 +131,7 @@ def test_delete_no_group(self): info = MagicMock(return_value=False) with patch.object(win_groupadd, 'info', info), \ patch.object(win_groupadd, '_get_computer_object', Mock()): - self.assertTrue(win_groupadd.delete('foo')) + self.assertFalse(win_groupadd.delete('foo')) def test_delete_error(self): ''' @@ -199,7 +199,7 @@ def test_adduser_already_exists(self): obj_group_mock = MagicMock(return_value=MockGroupObj('foo', ['WinNT://HOST/steve'])) with patch.object(win_groupadd, '_get_group_object', obj_group_mock), \ patch.object(salt.utils.win_functions, 'get_sam_name', sam_mock): - self.assertTrue(win_groupadd.adduser('foo', 'steve')) + self.assertFalse(win_groupadd.adduser('foo', 'steve')) def test_adduser_error(self): ''' @@ -227,7 +227,8 @@ def test_deluser(self): ''' # Test removing a user obj_group_mock = MagicMock(return_value=MockGroupObj('foo', ['WinNT://HOST/spongebob'])) - with patch.object(win_groupadd, '_get_group_object', obj_group_mock): + with patch.object(win_groupadd, '_get_group_object', obj_group_mock), \ + patch.object(salt.utils.win_functions, 'get_sam_name', sam_mock): self.assertTrue(win_groupadd.deluser('foo', 'spongebob')) def test_deluser_no_user(self): @@ -238,7 +239,7 @@ def test_deluser_no_user(self): obj_group_mock = MagicMock(return_value=MockGroupObj('foo', ['WinNT://HOST/steve'])) with patch.object(win_groupadd, '_get_group_object', obj_group_mock), \ patch.object(salt.utils.win_functions, 'get_sam_name', sam_mock): - self.assertTrue(win_groupadd.deluser('foo', 'spongebob')) + self.assertFalse(win_groupadd.deluser('foo', 'spongebob')) def test_deluser_error(self): ''' @@ -251,7 +252,7 @@ def Remove(self, name): obj_group_mock = MagicMock(return_value=GroupObj('foo', ['WinNT://HOST/spongebob'])) with patch.object(win_groupadd, '_get_group_object', obj_group_mock), \ patch.object(salt.utils.win_functions, 'get_sam_name', sam_mock): - self.assertTrue(win_groupadd.deluser('foo', 'spongebob')) + self.assertFalse(win_groupadd.deluser('foo', 'spongebob')) def test_deluser_group_does_not_exist(self): obj_group_mock = MagicMock(side_effect=PYWINTYPES_ERROR) From 0d32cb9228d3abaf69ab2f3077ed4817bf1e665f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= Date: Tue, 18 Dec 2018 15:44:28 +0100 Subject: [PATCH 44/55] libvirt events: fix domain defined/updated event details Libvirt events algorigthm converting the libvirt enums into string has a flaw that unit tests couldn't see. Libvirt python binding defines the following constants: VIR_DOMAIN_EVENT_CRASHED_PANICKED = 0 VIR_DOMAIN_EVENT_DEFINED = 0 VIR_DOMAIN_EVENT_CRASHED = 8 However VIR_DOMAIN_EVENT_CRASHED_PANICKED is the only value in this enum and thus wasn't not considered a sub-enum element. So the value 0 in enum 'VIR_DOMAIN_EVENT_' was wrongly mapped to "crashed panicked" instead of "defined". In order to safely rule this case out, check if we have an item that just ends with the subprefix without the '_'. --- salt/engines/libvirt_events.py | 2 +- tests/unit/engines/test_libvirt_events.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/salt/engines/libvirt_events.py b/salt/engines/libvirt_events.py index 117228b72517..69162f6176b8 100644 --- a/salt/engines/libvirt_events.py +++ b/salt/engines/libvirt_events.py @@ -165,7 +165,7 @@ def _get_libvirt_enum_string(prefix, value): # Filter out the values starting with a common base as they match another enum prefixes = [_compute_subprefix(p) for p in attributes] counts = {p: prefixes.count(p) for p in prefixes} - sub_prefixes = [p for p, count in counts.items() if count > 1] + sub_prefixes = [p for p, count in counts.items() if count > 1 or (p.endswith('_') and p[:-1] in prefixes)] filtered = [attr for attr in attributes if _compute_subprefix(attr) not in sub_prefixes] for candidate in filtered: diff --git a/tests/unit/engines/test_libvirt_events.py b/tests/unit/engines/test_libvirt_events.py index 1b03aad1c2ca..223b1b5d68cf 100644 --- a/tests/unit/engines/test_libvirt_events.py +++ b/tests/unit/engines/test_libvirt_events.py @@ -56,18 +56,19 @@ def test_get_libvirt_enum_string_subprefix(self, libvirt_mock): @patch('salt.engines.libvirt_events.libvirt', VIR_PREFIX_FOO=0, - VIR_PREFIX_FOO_BAR=1, - VIR_PREFIX_BAR_FOO=2) + VIR_PREFIX_BAR_FOO=1) def test_get_libvirt_enum_string_underscores(self, libvirt_mock): ''' Make sure the libvirt enum value to string works reliably and items with an underscore aren't confused with sub prefixes. ''' - assert libvirt_events._get_libvirt_enum_string('VIR_PREFIX_', 1) == 'foo bar' + assert libvirt_events._get_libvirt_enum_string('VIR_PREFIX_', 1) == 'bar foo' @patch('salt.engines.libvirt_events.libvirt', + VIR_DOMAIN_EVENT_CRASHED_PANICKED=0, VIR_DOMAIN_EVENT_DEFINED=0, VIR_DOMAIN_EVENT_UNDEFINED=1, + VIR_DOMAIN_EVENT_CRASHED=2, VIR_DOMAIN_EVENT_DEFINED_ADDED=0, VIR_DOMAIN_EVENT_DEFINED_UPDATED=1) def test_get_domain_event_detail(self, mock_libvirt): From 1672546d4c22cdb54c492d96c353c2437a482753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= Date: Tue, 18 Dec 2018 15:52:41 +0100 Subject: [PATCH 45/55] virt update/init: update current memory with memory Setting the element in the libvirt definition just changes the maximum amount of memory. Also set the amount to the same value. --- salt/modules/virt.py | 11 ++++++----- salt/templates/virt/libvirt_domain.jinja | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/salt/modules/virt.py b/salt/modules/virt.py index a96f6de5ba43..bb8f661e42bb 100644 --- a/salt/modules/virt.py +++ b/salt/modules/virt.py @@ -1786,11 +1786,12 @@ def update(name, need_update = True # Update the memory, note that libvirt outputs all memory sizes in KiB - mem_node = desc.find('memory') - if mem and int(mem_node.text) != mem * 1024: - mem_node.text = six.text_type(mem) - mem_node.set('unit', 'MiB') - need_update = True + for mem_node_name in ['memory', 'currentMemory']: + mem_node = desc.find(mem_node_name) + if mem and int(mem_node.text) != mem * 1024: + mem_node.text = six.text_type(mem) + mem_node.set('unit', 'MiB') + need_update = True # Update the XML definition with the new disks and diff changes devices_node = desc.find('devices') diff --git a/salt/templates/virt/libvirt_domain.jinja b/salt/templates/virt/libvirt_domain.jinja index 2e87ba455934..0b4c3fc2d6eb 100644 --- a/salt/templates/virt/libvirt_domain.jinja +++ b/salt/templates/virt/libvirt_domain.jinja @@ -2,6 +2,7 @@ {{ name }} {{ cpu }} {{ mem }} + {{ mem }} {{ os_type }} {% if kernel %}{{ kernel }}{% endif %} From f81d66d53a7e100c2a233bcbd963b863fbb1ff05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= Date: Tue, 18 Dec 2018 16:17:22 +0100 Subject: [PATCH 46/55] pylint: fix libvirt_events.py Fix unused variable in _domain_event_graphics_cb. --- salt/engines/libvirt_events.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/salt/engines/libvirt_events.py b/salt/engines/libvirt_events.py index 69162f6176b8..6d55fba86100 100644 --- a/salt/engines/libvirt_events.py +++ b/salt/engines/libvirt_events.py @@ -313,10 +313,9 @@ def get_address(addr): ''' transform address structure into event data piece ''' - data = {'family': _get_libvirt_enum_string('{0}_ADDRESS_'.format(prefix), addr['family']), + return {'family': _get_libvirt_enum_string('{0}_ADDRESS_'.format(prefix), addr['family']), 'node': addr['node'], 'service': addr['service']} - return addr _salt_send_domain_event(opaque, conn, domain, opaque['event'], { 'phase': _get_libvirt_enum_string(prefix, phase), From a5dffb9efed5adc98771839d151e2e567bb4d247 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Tue, 18 Dec 2018 12:51:06 -0700 Subject: [PATCH 47/55] Increase and standardize ShellCase timeouts --- tests/support/case.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/support/case.py b/tests/support/case.py index d9e35413ffe7..f721320c9b54 100644 --- a/tests/support/case.py +++ b/tests/support/case.py @@ -480,6 +480,7 @@ class ShellCase(ShellTestCase, AdaptedConfigurationTestCaseMixin, ScriptPathMixi _code_dir_ = CODE_DIR _script_dir_ = SCRIPT_DIR _python_executable_ = PYEXEC + RUN_TIMEOUT = 500 def chdir(self, dirname): try: @@ -487,7 +488,8 @@ def chdir(self, dirname): except OSError: os.chdir(INTEGRATION_TEST_DIR) - def run_salt(self, arg_str, with_retcode=False, catch_stderr=False, timeout=90, popen_kwargs=None): # pylint: disable=W0221 + def run_salt(self, arg_str, with_retcode=False, catch_stderr=False, + timeout=RUN_TIMEOUT, popen_kwargs=None): # pylint: disable=W0221 ''' Execute salt ''' @@ -501,7 +503,7 @@ def run_salt(self, arg_str, with_retcode=False, catch_stderr=False, timeout=90, log.debug('Result of run_salt for command \'%s\': %s', arg_str, ret) return ret - def run_spm(self, arg_str, with_retcode=False, catch_stderr=False, timeout=60): # pylint: disable=W0221 + def run_spm(self, arg_str, with_retcode=False, catch_stderr=False, timeout=RUN_TIMEOUT): # pylint: disable=W0221 ''' Execute spm ''' @@ -514,7 +516,7 @@ def run_spm(self, arg_str, with_retcode=False, catch_stderr=False, timeout=60): return ret def run_ssh(self, arg_str, with_retcode=False, catch_stderr=False, # pylint: disable=W0221 - timeout=60, wipe=True, raw=False): + timeout=RUN_TIMEOUT, wipe=True, raw=False): ''' Execute salt-ssh ''' @@ -535,7 +537,7 @@ def run_ssh(self, arg_str, with_retcode=False, catch_stderr=False, # pylint: di return ret def run_run(self, arg_str, with_retcode=False, catch_stderr=False, - asynchronous=False, timeout=180, config_dir=None, **kwargs): + asynchronous=False, timeout=RUN_TIMEOUT, config_dir=None, **kwargs): ''' Execute salt-run ''' @@ -592,7 +594,7 @@ def run_run_plus(self, fun, *arg, **kwargs): fun, opts_arg, ret) return ret - def run_key(self, arg_str, catch_stderr=False, with_retcode=False): + def run_key(self, arg_str, catch_stderr=False, with_retcode=False, timeout=RUN_TIMEOUT): ''' Execute salt-key ''' @@ -601,11 +603,11 @@ def run_key(self, arg_str, catch_stderr=False, with_retcode=False): arg_str, catch_stderr=catch_stderr, with_retcode=with_retcode, - timeout=60) + timeout=timeout) log.debug('Result of run_key for command \'%s\': %s', arg_str, ret) return ret - def run_cp(self, arg_str, with_retcode=False, catch_stderr=False): + def run_cp(self, arg_str, with_retcode=False, catch_stderr=False, timeout=RUN_TIMEOUT): ''' Execute salt-cp ''' @@ -616,9 +618,9 @@ def run_cp(self, arg_str, with_retcode=False, catch_stderr=False): arg_str, with_retcode=with_retcode, catch_stderr=catch_stderr, - timeout=60) + timeout=timeout) - def run_call(self, arg_str, with_retcode=False, catch_stderr=False, local=False): + def run_call(self, arg_str, with_retcode=False, catch_stderr=False, local=False, timeout=RUN_TIMEOUT): ''' Execute salt-call. ''' @@ -628,11 +630,11 @@ def run_call(self, arg_str, with_retcode=False, catch_stderr=False, local=False) arg_str, with_retcode=with_retcode, catch_stderr=catch_stderr, - timeout=60) + timeout=timeout) log.debug('Result of run_call for command \'%s\': %s', arg_str, ret) return ret - def run_cloud(self, arg_str, catch_stderr=False, timeout=30): + def run_cloud(self, arg_str, catch_stderr=False, timeout=RUN_TIMEOUT): ''' Execute salt-cloud ''' From daffcb3b6a4aaac6e377d15b8ac437fb3da4d493 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Tue, 18 Dec 2018 13:05:45 -0700 Subject: [PATCH 48/55] Fix linter warnings --- tests/support/case.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/support/case.py b/tests/support/case.py index f721320c9b54..3a8c787c495b 100644 --- a/tests/support/case.py +++ b/tests/support/case.py @@ -488,8 +488,8 @@ def chdir(self, dirname): except OSError: os.chdir(INTEGRATION_TEST_DIR) - def run_salt(self, arg_str, with_retcode=False, catch_stderr=False, - timeout=RUN_TIMEOUT, popen_kwargs=None): # pylint: disable=W0221 + def run_salt(self, arg_str, with_retcode=False, catch_stderr=False, # pylint: disable=W0221 + timeout=RUN_TIMEOUT, popen_kwargs=None): ''' Execute salt ''' @@ -594,7 +594,8 @@ def run_run_plus(self, fun, *arg, **kwargs): fun, opts_arg, ret) return ret - def run_key(self, arg_str, catch_stderr=False, with_retcode=False, timeout=RUN_TIMEOUT): + def run_key(self, arg_str, catch_stderr=False, with_retcode=False, # pylint: disable=W0221 + timeout=RUN_TIMEOUT): ''' Execute salt-key ''' @@ -607,7 +608,8 @@ def run_key(self, arg_str, catch_stderr=False, with_retcode=False, timeout=RUN_T log.debug('Result of run_key for command \'%s\': %s', arg_str, ret) return ret - def run_cp(self, arg_str, with_retcode=False, catch_stderr=False, timeout=RUN_TIMEOUT): + def run_cp(self, arg_str, with_retcode=False, catch_stderr=False, # pylint: disable=W0221 + timeout=RUN_TIMEOUT): ''' Execute salt-cp ''' @@ -620,7 +622,8 @@ def run_cp(self, arg_str, with_retcode=False, catch_stderr=False, timeout=RUN_TI catch_stderr=catch_stderr, timeout=timeout) - def run_call(self, arg_str, with_retcode=False, catch_stderr=False, local=False, timeout=RUN_TIMEOUT): + def run_call(self, arg_str, with_retcode=False, catch_stderr=False, # pylint: disable=W0221 + local=False, timeout=RUN_TIMEOUT): ''' Execute salt-call. ''' From f06898e85d7b9656cd1c6fb038d39519d5f508c3 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Wed, 19 Dec 2018 09:52:01 -0700 Subject: [PATCH 49/55] Do not pass unicode in environment --- salt/utils/gitfs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index c3127ff4621a..1cc06fcb09cd 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -590,8 +590,8 @@ def clean_stale_refs(self): # According to stackoverflow (http://goo.gl/l74GC8), we are setting LANGUAGE as well # just to be sure. env = os.environ.copy() - env["LANGUAGE"] = "C" - env["LC_ALL"] = "C" + env[b"LANGUAGE"] = b"C" + env[b"LC_ALL"] = b"C" cmd = subprocess.Popen( shlex.split(cmd_str), From 7ba54cd7b2dabb01fb3cdd1e8b3cbe2361d4f988 Mon Sep 17 00:00:00 2001 From: twangboy Date: Wed, 19 Dec 2018 10:05:03 -0700 Subject: [PATCH 50/55] Fix `test_debian_ip` on Windows --- tests/unit/modules/test_debian_ip.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/modules/test_debian_ip.py b/tests/unit/modules/test_debian_ip.py index a49cda9f70d0..9b59d310fe63 100644 --- a/tests/unit/modules/test_debian_ip.py +++ b/tests/unit/modules/test_debian_ip.py @@ -6,6 +6,7 @@ # Import Python libs from __future__ import absolute_import, print_function, unicode_literals import tempfile +import os # Import Salt Testing Libs from tests.support.mixins import LoaderModuleMockMixin @@ -438,7 +439,7 @@ def test_build_interface(self): '\n']}, ] - with tempfile.NamedTemporaryFile(mode='r', delete=True) as tfile: + with tempfile.NamedTemporaryFile(mode='r', delete=False) as tfile: with patch('salt.modules.debian_ip._DEB_NETWORK_FILE', str(tfile.name)): for iface in interfaces: # Skip tests that require __salt__['pkg.install']() @@ -450,6 +451,7 @@ def test_build_interface(self): interface_file=tfile.name, **iface['settings']), iface['return']) + os.remove(tfile.name) # 'up' function tests: 1 From bbc12446a876361f8aba446bf68f16477439aaff Mon Sep 17 00:00:00 2001 From: twangboy Date: Wed, 19 Dec 2018 10:14:02 -0700 Subject: [PATCH 51/55] Skip debian tests on Windows --- tests/unit/modules/test_debian_ip.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/modules/test_debian_ip.py b/tests/unit/modules/test_debian_ip.py index 9b59d310fe63..8a6c957c1b69 100644 --- a/tests/unit/modules/test_debian_ip.py +++ b/tests/unit/modules/test_debian_ip.py @@ -6,7 +6,6 @@ # Import Python libs from __future__ import absolute_import, print_function, unicode_literals import tempfile -import os # Import Salt Testing Libs from tests.support.mixins import LoaderModuleMockMixin @@ -20,12 +19,14 @@ # Import Salt Libs import salt.modules.debian_ip as debian_ip +import salt.utils.platform # Import third party libs import jinja2.exceptions @skipIf(NO_MOCK, NO_MOCK_REASON) +@skipIf(salt.utils.platform.is_windows(), 'Do not run these tests on Windows') class DebianIpTestCase(TestCase, LoaderModuleMockMixin): ''' Test cases for salt.modules.debian_ip @@ -439,7 +440,7 @@ def test_build_interface(self): '\n']}, ] - with tempfile.NamedTemporaryFile(mode='r', delete=False) as tfile: + with tempfile.NamedTemporaryFile(mode='r', delete=True) as tfile: with patch('salt.modules.debian_ip._DEB_NETWORK_FILE', str(tfile.name)): for iface in interfaces: # Skip tests that require __salt__['pkg.install']() @@ -451,7 +452,6 @@ def test_build_interface(self): interface_file=tfile.name, **iface['settings']), iface['return']) - os.remove(tfile.name) # 'up' function tests: 1 From db2a1cc016ff8fcc1530161797522ad3bfd56b50 Mon Sep 17 00:00:00 2001 From: twangboy Date: Wed, 19 Dec 2018 22:32:33 -0700 Subject: [PATCH 52/55] Fix failing tests on Py3 These should have been failing across the board. I don't know what's going on with the tests suite --- tests/unit/utils/test_win_lgpo_netsh.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/utils/test_win_lgpo_netsh.py b/tests/unit/utils/test_win_lgpo_netsh.py index b7098b0e0dc3..f268598d142b 100644 --- a/tests/unit/utils/test_win_lgpo_netsh.py +++ b/tests/unit/utils/test_win_lgpo_netsh.py @@ -335,6 +335,7 @@ def test_set_firewall_logging_maxfilesize_local(self): @destructiveTest def test_set_firewall_settings_fwrules_local_enable(self): self.assertRaises( + CommandExecutionError, win_lgpo_netsh.set_settings, profile='domain', setting='localfirewallrules', @@ -366,6 +367,7 @@ def test_set_firewall_settings_fwrules_lgpo_notconfigured(self): @destructiveTest def test_set_firewall_settings_consecrules_local_enable(self): self.assertRaises( + CommandExecutionError, win_lgpo_netsh.set_settings, profile='domain', setting='localconsecrules', From 08cb05d4488770d5de05156cb4e6021a2072bed7 Mon Sep 17 00:00:00 2001 From: Mircea Ulinic Date: Thu, 20 Dec 2018 13:44:11 +0000 Subject: [PATCH 53/55] #50924: Check if running a NAPALM Proxy or regular Minion --- salt/utils/napalm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/utils/napalm.py b/salt/utils/napalm.py index b3377f7b963e..73c815970b0d 100644 --- a/salt/utils/napalm.py +++ b/salt/utils/napalm.py @@ -95,7 +95,7 @@ def virtual(opts, virtualname, filename): ''' Returns the __virtual__. ''' - if (HAS_NAPALM and NAPALM_MAJOR >= 2) or HAS_NAPALM_BASE: + if ((HAS_NAPALM and NAPALM_MAJOR >= 2) or HAS_NAPALM_BASE) and (is_proxy(opts) or is_minion(opts)): return virtualname else: return ( From d661b6ea28fb3530de09015300693b3e141feffe Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Mon, 24 Dec 2018 16:38:20 -0800 Subject: [PATCH 54/55] Fixing def test_build_interface in merge-forward --- tests/unit/modules/test_debian_ip.py | 308 +-------------------------- 1 file changed, 3 insertions(+), 305 deletions(-) diff --git a/tests/unit/modules/test_debian_ip.py b/tests/unit/modules/test_debian_ip.py index 8d7110861024..dd2088dde6fd 100644 --- a/tests/unit/modules/test_debian_ip.py +++ b/tests/unit/modules/test_debian_ip.py @@ -7,6 +7,7 @@ from __future__ import absolute_import, print_function, unicode_literals from collections import OrderedDict as odict import tempfile +import os # Import Salt Testing Libs from tests.support.mixins import LoaderModuleMockMixin @@ -941,7 +942,7 @@ def test_build_interface(self): self.assertTrue(debian_ip.build_interface('eth0', 'eth', 'enabled', test='True')) - with tempfile.NamedTemporaryFile(mode='r', delete=True) as tfile: + with tempfile.NamedTemporaryFile(mode='r', delete=False) as tfile: with patch('salt.modules.debian_ip._DEB_NETWORK_FILE', str(tfile.name)): for iface in test_interfaces: if iface.get('skip_test', False): @@ -957,310 +958,7 @@ def test_build_interface(self): interface_file=tfile.name, **iface['build_interface']), iface['return']) - - # 'build_interface' function tests: 1 - - def test_build_interface(self): - ''' - Test if it builds an interface script for a network interface. - ''' - with patch('salt.modules.debian_ip._write_file_ifaces', - MagicMock(return_value='salt')): - self.assertEqual(debian_ip.build_interface('eth0', 'eth', 'enabled'), - ['s\n', 'a\n', 'l\n', 't\n']) - - self.assertTrue(debian_ip.build_interface('eth0', 'eth', 'enabled', - test='True')) - - with patch.object(debian_ip, '_parse_settings_eth', - MagicMock(return_value={'routes': []})): - self.assertRaises(AttributeError, debian_ip.build_interface, - 'eth0', 'bridge', 'enabled') - - self.assertRaises(AttributeError, debian_ip.build_interface, - 'eth0', 'slave', 'enabled') - - self.assertRaises(AttributeError, debian_ip.build_interface, - 'eth0', 'bond', 'enabled') - - self.assertTrue(debian_ip.build_interface('eth0', 'eth', 'enabled', - test='True')) - - interfaces = [ - # IPv4-only interface; single address - {'iface_name': 'eth1', 'iface_type': 'eth', 'enabled': True, - 'settings': { - 'proto': 'static', - 'ipaddr': '192.168.4.9', - 'netmask': '255.255.255.0', - 'gateway': '192.168.4.1', - 'enable_ipv6': False, - 'noifupdown': True, - }, - 'return': [ - 'auto eth1\n', - 'iface eth1 inet static\n', - ' address 192.168.4.9\n', - ' netmask 255.255.255.0\n', - ' gateway 192.168.4.1\n', - '\n']}, - # IPv6-only; single address - {'iface_name': 'eth2', 'iface_type': 'eth', 'enabled': True, - 'settings': { - 'proto': 'static', - 'ipv6proto': 'static', - 'ipv6ipaddr': '2001:db8:dead:beef::3', - 'ipv6netmask': '64', - 'ipv6gateway': '2001:db8:dead:beef::1', - 'enable_ipv6': True, - 'noifupdown': True, - }, - 'return': [ - 'auto eth2\n', - 'iface eth2 inet6 static\n', - ' address 2001:db8:dead:beef::3\n', - ' netmask 64\n', - ' gateway 2001:db8:dead:beef::1\n', - '\n']}, - # IPv4 and IPv6; shared/overridden settings - {'iface_name': 'eth3', 'iface_type': 'eth', 'enabled': True, - 'settings': { - 'proto': 'static', - 'ipaddr': '192.168.4.9', - 'netmask': '255.255.255.0', - 'gateway': '192.168.4.1', - 'ipv6proto': 'static', - 'ipv6ipaddr': '2001:db8:dead:beef::3', - 'ipv6netmask': '64', - 'ipv6gateway': '2001:db8:dead:beef::1', - 'ttl': '18', # shared - 'ipv6ttl': '15', # overriden for v6 - 'mtu': '1480', # shared - 'enable_ipv6': True, - 'noifupdown': True, - }, - 'return': [ - 'auto eth3\n', - 'iface eth3 inet static\n', - ' address 192.168.4.9\n', - ' netmask 255.255.255.0\n', - ' gateway 192.168.4.1\n', - ' ttl 18\n', - ' mtu 1480\n', - 'iface eth3 inet6 static\n', - ' address 2001:db8:dead:beef::3\n', - ' netmask 64\n', - ' gateway 2001:db8:dead:beef::1\n', - ' ttl 15\n', - ' mtu 1480\n', - '\n']}, - # Slave iface - {'iface_name': 'eth4', 'iface_type': 'slave', 'enabled': True, - 'settings': { - 'master': 'bond0', - 'noifupdown': True, - }, - 'return': [ - 'auto eth4\n', - 'iface eth4 inet manual\n', - ' bond-master bond0\n', - '\n']}, - # Bond; with address IPv4 and IPv6 address; slaves as string - {'iface_name': 'bond5', 'iface_type': 'bond', 'enabled': True, - 'settings': { - 'proto': 'static', - 'ipaddr': '10.1.0.14', - 'netmask': '255.255.255.0', - 'gateway': '10.1.0.1', - 'ipv6proto': 'static', - 'ipv6ipaddr': '2001:db8:dead:c0::3', - 'ipv6netmask': '64', - 'ipv6gateway': '2001:db8:dead:c0::1', - 'mode': '802.3ad', - 'slaves': 'eth4 eth5', - 'enable_ipv6': True, - 'noifupdown': True, - }, - 'return': [ - 'auto bond5\n', - 'iface bond5 inet static\n', - ' address 10.1.0.14\n', - ' netmask 255.255.255.0\n', - ' gateway 10.1.0.1\n', - ' bond-ad_select 0\n', - ' bond-downdelay 200\n', - ' bond-lacp_rate 0\n', - ' bond-miimon 100\n', - ' bond-mode 4\n', - ' bond-slaves eth4 eth5\n', - ' bond-updelay 0\n', - ' bond-use_carrier on\n', - 'iface bond5 inet6 static\n', - ' address 2001:db8:dead:c0::3\n', - ' netmask 64\n', - ' gateway 2001:db8:dead:c0::1\n', - # TODO: I suspect there should be more here. - '\n']}, - # Bond; with address IPv4 and IPv6 address; slaves as list - {'iface_name': 'bond6', 'iface_type': 'bond', 'enabled': True, - 'settings': { - 'proto': 'static', - 'ipaddr': '10.1.0.14', - 'netmask': '255.255.255.0', - 'gateway': '10.1.0.1', - 'ipv6proto': 'static', - 'ipv6ipaddr': '2001:db8:dead:c0::3', - 'ipv6netmask': '64', - 'ipv6gateway': '2001:db8:dead:c0::1', - 'mode': '802.3ad', - # TODO: Need to add this support - #'slaves': ['eth4', 'eth5'], - 'slaves': 'eth4 eth5', - 'enable_ipv6': True, - 'noifupdown': True, - }, - 'return': [ - 'auto bond6\n', - 'iface bond6 inet static\n', - ' address 10.1.0.14\n', - ' netmask 255.255.255.0\n', - ' gateway 10.1.0.1\n', - ' bond-ad_select 0\n', - ' bond-downdelay 200\n', - ' bond-lacp_rate 0\n', - ' bond-miimon 100\n', - ' bond-mode 4\n', - ' bond-slaves eth4 eth5\n', - ' bond-updelay 0\n', - ' bond-use_carrier on\n', - 'iface bond6 inet6 static\n', - ' address 2001:db8:dead:c0::3\n', - ' netmask 64\n', - ' gateway 2001:db8:dead:c0::1\n', - # TODO: I suspect there should be more here. - '\n']}, - # Bond VLAN; with IPv4 address - {'iface_name': 'bond1.7', 'iface_type': 'vlan', 'enabled': True, - 'settings': { - 'proto': 'static', - 'ipaddr': '10.7.0.8', - 'netmask': '255.255.255.0', - 'gateway': '10.7.0.1', - 'slaves': 'eth6 eth7', - 'mode': '802.3ad', - 'enable_ipv6': False, - 'noifupdown': True, - }, - 'return': [ - 'auto bond1.7\n', - 'iface bond1.7 inet static\n', - ' vlan-raw-device bond1\n', - ' address 10.7.0.8\n', - ' netmask 255.255.255.0\n', - ' gateway 10.7.0.1\n', - ' mode 802.3ad\n', - '\n']}, - # Bond; without address - {'iface_name': 'bond1.8', 'iface_type': 'vlan', 'enabled': True, - 'settings': { - 'proto': 'static', - 'slaves': 'eth6 eth7', - 'mode': '802.3ad', - 'enable_ipv6': False, - 'noifupdown': True, - }, - 'return': [ - 'auto bond1.8\n', - 'iface bond1.8 inet static\n', - ' vlan-raw-device bond1\n', - ' mode 802.3ad\n', - '\n']}, - # DNS NS as list - {'iface_name': 'eth9', 'iface_type': 'eth', 'enabled': True, - 'settings': { - 'proto': 'static', - 'ipaddr': '192.168.4.9', - 'netmask': '255.255.255.0', - 'gateway': '192.168.4.1', - 'enable_ipv6': False, - 'noifupdown': True, - 'dns': ['8.8.8.8', '8.8.4.4'], - }, - 'return': [ - 'auto eth9\n', - 'iface eth9 inet static\n', - ' address 192.168.4.9\n', - ' netmask 255.255.255.0\n', - ' gateway 192.168.4.1\n', - ' dns-nameservers 8.8.8.8 8.8.4.4\n', - '\n']}, - # DNS NS as string - {'iface_name': 'eth10', 'iface_type': 'eth', 'enabled': True, - 'settings': { - 'proto': 'static', - 'ipaddr': '192.168.4.9', - 'netmask': '255.255.255.0', - 'gateway': '192.168.4.1', - 'enable_ipv6': False, - 'noifupdown': True, - 'dns': '8.8.8.8 8.8.4.4', - }, - 'return': [ - 'auto eth10\n', - 'iface eth10 inet static\n', - ' address 192.168.4.9\n', - ' netmask 255.255.255.0\n', - ' gateway 192.168.4.1\n', - ' dns-nameservers 8.8.8.8 8.8.4.4\n', - '\n']}, - # Loopback; with IPv4 and IPv6 address - {'iface_name': 'lo11', 'iface_type': 'eth', 'enabled': True, - 'settings': { - 'proto': 'loopback', - 'ipaddr': '192.168.4.9', - 'netmask': '255.255.255.0', - 'gateway': '192.168.4.1', - 'ipv6ipaddr': 'fc00::1', - 'ipv6netmask': '128', - 'ipv6_autoconf': False, - 'enable_ipv6': True, - 'noifupdown': True, - }, - 'return': [ - 'auto lo11\n', - 'iface lo11 inet loopback\n', - ' address 192.168.4.9\n', - ' netmask 255.255.255.0\n', - ' gateway 192.168.4.1\n', - 'iface lo11 inet6 loopback\n', - ' address fc00::1\n', - ' netmask 128\n', - '\n']}, - # Loopback; without address - {'iface_name': 'lo12', 'iface_type': 'eth', 'enabled': True, - 'settings': { - 'proto': 'loopback', - 'enable_ipv6': False, - 'noifupdown': True, - }, - 'return': [ - 'auto lo12\n', - 'iface lo12 inet loopback\n', - '\n']}, - ] - - with tempfile.NamedTemporaryFile(mode='r', delete=True) as tfile: - with patch('salt.modules.debian_ip._DEB_NETWORK_FILE', str(tfile.name)): - for iface in interfaces: - # Skip tests that require __salt__['pkg.install']() - if iface['iface_type'] not in ['bridge', 'pppoe', 'vlan']: - self.assertListEqual(debian_ip.build_interface( - iface=iface['iface_name'], - iface_type=iface['iface_type'], - enabled=iface['enabled'], - interface_file=tfile.name, - **iface['settings']), - iface['return']) + os.remove(tfile.name) # 'up' function tests: 1 From 01b4f6f4729331145aba3453bd8254e0d65789c0 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Sat, 29 Dec 2018 11:03:44 -0800 Subject: [PATCH 55/55] Ensure the refresh_pillar is run against both the minion and sub_minion, otherwise minion_blackout pillar value is left behind for the sub_minion. --- tests/integration/minion/test_blackout.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/minion/test_blackout.py b/tests/integration/minion/test_blackout.py index 90bc79799bf6..381b617d920c 100644 --- a/tests/integration/minion/test_blackout.py +++ b/tests/integration/minion/test_blackout.py @@ -32,7 +32,8 @@ def begin_blackout(self, blackout_data='minion_blackout: True'): ''' with salt.utils.files.fopen(BLACKOUT_PILLAR, 'w') as wfh: wfh.write(blackout_data) - self.run_function('saltutil.refresh_pillar') + self.run_function('saltutil.refresh_pillar', minion_tgt='minion') + self.run_function('saltutil.refresh_pillar', minion_tgt='sub_minion') sleep(10) # wait for minion to enter blackout mode def end_blackout(self): @@ -43,7 +44,8 @@ def end_blackout(self): blackout_pillar.write(textwrap.dedent('''\ minion_blackout: False ''')) - self.run_function('saltutil.refresh_pillar') + self.run_function('saltutil.refresh_pillar', minion_tgt='minion') + self.run_function('saltutil.refresh_pillar', minion_tgt='sub_minion') sleep(10) # wait for minion to exit blackout mode @flaky