From c3ce53e6d617af6f45aef92bacf89ba30f9099ac Mon Sep 17 00:00:00 2001 From: Joachim Gleissner Date: Fri, 1 Feb 2019 18:33:29 +0100 Subject: [PATCH 01/33] Do not treat MSI auth support as hard dependency On systems that lack MSI authentication support, only fail if MSI is actually used. This allows users on older versions of msrestazure to use the azurearm back-end. --- salt/cloud/__init__.py | 2 +- salt/utils/azurearm.py | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/salt/cloud/__init__.py b/salt/cloud/__init__.py index a2ae81445dbf..6201f7af1b60 100644 --- a/salt/cloud/__init__.py +++ b/salt/cloud/__init__.py @@ -74,7 +74,7 @@ def _call(queue, args, kwargs): queue.put("KEYBOARDINT") queue.put("Keyboard interrupt") queue.put("{0}\n{1}\n".format(ex, trace)) - except Exception as ex: # pylint: disable=broad-except + except BaseException as ex: trace = traceback.format_exc() queue.put("ERROR") queue.put("Exception") diff --git a/salt/utils/azurearm.py b/salt/utils/azurearm.py index 2dbcfe4ec6cf..96055aa9c433 100644 --- a/salt/utils/azurearm.py +++ b/salt/utils/azurearm.py @@ -47,7 +47,6 @@ UserPassCredentials, ServicePrincipalCredentials, ) - from msrestazure.azure_active_directory import MSIAuthentication from msrestazure.azure_cloud import ( MetadataEndpointError, get_cloud_from_metadata_endpoint, @@ -123,7 +122,14 @@ def _determine_auth(**kwargs): kwargs["username"], kwargs["password"], cloud_environment=cloud_env ) elif "subscription_id" in kwargs: - credentials = MSIAuthentication(cloud_environment=cloud_env) + try: + from msrestazure.azure_active_directory import MSIAuthentication + + credentials = MSIAuthentication(cloud_environment=cloud_env) + except ImportError: + raise SaltSystemExit( + msg="MSI authentication support not availabe (requires msrestazure >= 0.4.14)" + ) else: raise SaltInvocationError( @@ -161,7 +167,7 @@ def get_client(client_type, **kwargs): if client_type not in client_map: raise SaltSystemExit( - "The Azure ARM client_type {0} specified can not be found.".format( + msg="The Azure ARM client_type {0} specified can not be found.".format( client_type ) ) From ee53f3172d94217ab25ff2e85ed23522d168f60c Mon Sep 17 00:00:00 2001 From: Joachim Gleissner Date: Thu, 7 Feb 2019 14:58:14 +0100 Subject: [PATCH 02/33] catch SystemError instead of BaseException --- salt/cloud/__init__.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/salt/cloud/__init__.py b/salt/cloud/__init__.py index 6201f7af1b60..8adb82f29f2d 100644 --- a/salt/cloud/__init__.py +++ b/salt/cloud/__init__.py @@ -74,11 +74,17 @@ def _call(queue, args, kwargs): queue.put("KEYBOARDINT") queue.put("Keyboard interrupt") queue.put("{0}\n{1}\n".format(ex, trace)) - except BaseException as ex: + except Exception as ex: trace = traceback.format_exc() queue.put("ERROR") queue.put("Exception") queue.put("{0}\n{1}\n".format(ex, trace)) + except SystemExit as ex: + print("Communicatior: caught exception") + trace = traceback.format_exc() + queue.put("ERROR") + queue.put("System exit") + queue.put("{0}\n{1}\n".format(ex, trace)) return ret return _call From 6bfaed9a7d6a9852e21e6f01d84ff221ffa59b46 Mon Sep 17 00:00:00 2001 From: Tom Williams Date: Thu, 28 Feb 2019 12:49:53 -0500 Subject: [PATCH 03/33] INFRA-5209 - Avoid throwing exception for missing security group in test mode --- salt/modules/boto_secgroup.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/salt/modules/boto_secgroup.py b/salt/modules/boto_secgroup.py index cff5a265be78..2d12bab46b99 100644 --- a/salt/modules/boto_secgroup.py +++ b/salt/modules/boto_secgroup.py @@ -398,10 +398,20 @@ def convert_to_group_ids( ) if not group_id: # Security groups are a big deal - need to fail if any can't be resolved... - raise CommandExecutionError( - "Could not resolve Security Group name " - "{0} to a Group ID".format(group) - ) + # But... if we're running in test mode, it may just be that the SG is scheduled + # to be created, and thus WOULD have been there if running "for real"... + if __opts__["test"]: + log.warn( + "Security Group `%s` could not be resolved to an ID. This may " + "cause a failure when not running in test mode.", + group, + ) + return [] + else: + raise CommandExecutionError( + "Could not resolve Security Group name " + "{0} to a Group ID".format(group) + ) else: group_ids.append(six.text_type(group_id)) log.debug("security group contents %s post-conversion", group_ids) From d307401059241d6ced7764a7795b16b82f29aac1 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Sun, 20 Jan 2019 09:18:59 -0600 Subject: [PATCH 04/33] boto: Replace some unnecessary usage of locals() This makes the code less misleading. Note that I've also changed the variable names used in dict iteration here, as `val` implies that it contains a value and it contains a dictionary _key_ instead. --- salt/states/boto_lambda.py | 41 ++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/salt/states/boto_lambda.py b/salt/states/boto_lambda.py index a416359af313..a65a1dd6eadd 100644 --- a/salt/states/boto_lambda.py +++ b/salt/states/boto_lambda.py @@ -429,23 +429,20 @@ def _function_config_present( func = __salt__["boto_lambda.describe_function"]( FunctionName, region=region, key=key, keyid=keyid, profile=profile )["function"] - # pylint: disable=possibly-unused-variable - role_arn = _get_role_arn(Role, region, key, keyid, profile) - # pylint: enable=possibly-unused-variable need_update = False options = { - "Role": "role_arn", - "Handler": "Handler", - "Description": "Description", - "Timeout": "Timeout", - "MemorySize": "MemorySize", + "Role": _get_role_arn(Role, region, key, keyid, profile), + "Handler": Handler, + "Description": Description, + "Timeout": Timeout, + "MemorySize": MemorySize, } - for val, var in six.iteritems(options): - if func[val] != locals()[var]: + for key, val in six.iteritems(options): + if func[key] != val: need_update = True - ret["changes"].setdefault("new", {})[var] = locals()[var] - ret["changes"].setdefault("old", {})[var] = func[val] + ret["changes"].setdefault("old", {})[key] = func[key] + ret["changes"].setdefault("new", {})[key] = val # VpcConfig returns the extra value 'VpcId' so do a special compare oldval = func.get("VpcConfig") if oldval is not None: @@ -787,13 +784,13 @@ def alias_present( )["alias"] need_update = False - options = {"FunctionVersion": "FunctionVersion", "Description": "Description"} + options = {"FunctionVersion": FunctionVersion, "Description": Description} - for val, var in six.iteritems(options): - if _describe[val] != locals()[var]: + for key, val in six.iteritems(options): + if _describe[key] != val: need_update = True - ret["changes"].setdefault("new", {})[var] = locals()[var] - ret["changes"].setdefault("old", {})[var] = _describe[val] + ret["changes"].setdefault("old", {})[key] = _describe[key] + ret["changes"].setdefault("new", {})[key] = val if need_update: ret["comment"] = os.linesep.join( [ret["comment"], "Alias config to be modified"] @@ -1026,13 +1023,13 @@ def event_source_mapping_present( )["event_source_mapping"] need_update = False - options = {"BatchSize": "BatchSize"} + options = {"BatchSize": BatchSize} - for val, var in six.iteritems(options): - if _describe[val] != locals()[var]: + for key, val in six.iteritems(options): + if _describe[key] != val: need_update = True - ret["changes"].setdefault("new", {})[var] = locals()[var] - ret["changes"].setdefault("old", {})[var] = _describe[val] + ret["changes"].setdefault("old", {})[key] = _describe[key] + ret["changes"].setdefault("new", {})[key] = val # verify FunctionName against FunctionArn function_arn = _get_function_arn( FunctionName, region=region, key=key, keyid=keyid, profile=profile From 97094859537ce154d635de41a044c448cffe2ad7 Mon Sep 17 00:00:00 2001 From: Joachim Gleissner Date: Wed, 13 Feb 2019 10:41:30 +0100 Subject: [PATCH 05/33] salt-cloud: removed debug print --- salt/cloud/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/salt/cloud/__init__.py b/salt/cloud/__init__.py index 8adb82f29f2d..3006bf6f5b8e 100644 --- a/salt/cloud/__init__.py +++ b/salt/cloud/__init__.py @@ -74,13 +74,12 @@ def _call(queue, args, kwargs): queue.put("KEYBOARDINT") queue.put("Keyboard interrupt") queue.put("{0}\n{1}\n".format(ex, trace)) - except Exception as ex: + except Exception as ex: # pylint: disable=broad-except trace = traceback.format_exc() queue.put("ERROR") queue.put("Exception") queue.put("{0}\n{1}\n".format(ex, trace)) except SystemExit as ex: - print("Communicatior: caught exception") trace = traceback.format_exc() queue.put("ERROR") queue.put("System exit") From 68208e0d44ea993c81861818b7484d3fc64b81b0 Mon Sep 17 00:00:00 2001 From: Tom Williams Date: Tue, 20 Nov 2018 23:06:20 +0000 Subject: [PATCH 06/33] INFRA-7855 - allow boto_lambda.function_present() to support remote URLs for ZipFile param --- salt/states/boto_lambda.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/salt/states/boto_lambda.py b/salt/states/boto_lambda.py index a416359af313..c5ce84a6dac1 100644 --- a/salt/states/boto_lambda.py +++ b/salt/states/boto_lambda.py @@ -508,6 +508,13 @@ def _function_code_present( )["function"] update = False if ZipFile: + if '://' in ZipFile: # Looks like a remote URL to me... + dlZipFile = __salt__['cp.cache_file'](path=ZipFile) + if dlZipFile is False: + ret['result'] = False + ret['comment'] = 'Failed to cache ZipFile `{0}`.'.format(ZipFile) + return ret + ZipFile = dlZipFile size = os.path.getsize(ZipFile) if size == func["CodeSize"]: sha = hashlib.sha256() From bcbe25b0135b0550f355b61ed3eca046372a3288 Mon Sep 17 00:00:00 2001 From: Tom Williams Date: Tue, 27 Nov 2018 23:27:01 +0000 Subject: [PATCH 07/33] INFRA-7855 - derp, didn't realize the create call used a completely different loop --- salt/modules/boto_lambda.py | 8 ++++++++ salt/states/boto_lambda.py | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/salt/modules/boto_lambda.py b/salt/modules/boto_lambda.py index 96e601141ea1..d1c0d41f94e6 100644 --- a/salt/modules/boto_lambda.py +++ b/salt/modules/boto_lambda.py @@ -264,6 +264,7 @@ def create_function( .. code-block:: bash salt myminion boto_lamba.create_function my_function python2.7 my_role my_file.my_function my_function.zip + salt myminion boto_lamba.create_function my_function python2.7 my_role my_file.my_function salt://files/my_function.zip """ @@ -276,6 +277,13 @@ def create_function( "Either ZipFile must be specified, or " "S3Bucket and S3Key must be provided." ) + if "://" in ZipFile: # Looks like a remote URL to me... + dlZipFile = __salt__["cp.cache_file"](path=ZipFile) + if dlZipFile is False: + ret["result"] = False + ret["comment"] = "Failed to cache ZipFile `{0}`.".format(ZipFile) + return ret + ZipFile = dlZipFile code = { "ZipFile": _filedata(ZipFile), } diff --git a/salt/states/boto_lambda.py b/salt/states/boto_lambda.py index c5ce84a6dac1..717fd5256b8e 100644 --- a/salt/states/boto_lambda.py +++ b/salt/states/boto_lambda.py @@ -508,11 +508,11 @@ def _function_code_present( )["function"] update = False if ZipFile: - if '://' in ZipFile: # Looks like a remote URL to me... - dlZipFile = __salt__['cp.cache_file'](path=ZipFile) + if "://" in ZipFile: # Looks like a remote URL to me... + dlZipFile = __salt__["cp.cache_file"](path=ZipFile) if dlZipFile is False: - ret['result'] = False - ret['comment'] = 'Failed to cache ZipFile `{0}`.'.format(ZipFile) + ret["result"] = False + ret["comment"] = "Failed to cache ZipFile `{0}`.".format(ZipFile) return ret ZipFile = dlZipFile size = os.path.getsize(ZipFile) From 9d21aeec98670629876d84ceebbeefdf9296d56b Mon Sep 17 00:00:00 2001 From: Tom Williams Date: Thu, 29 Nov 2018 23:03:04 +0000 Subject: [PATCH 08/33] INFRA-7855-3 - fix weird corner case in boto3_sns module :-/ --- salt/modules/boto3_sns.py | 19 +++++++++++++++++++ salt/states/boto3_sns.py | 4 +++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/salt/modules/boto3_sns.py b/salt/modules/boto3_sns.py index 6866219807b6..8a691865a66a 100644 --- a/salt/modules/boto3_sns.py +++ b/salt/modules/boto3_sns.py @@ -116,6 +116,14 @@ def describe_topic(name, region=None, key=None, keyid=None, profile=None): ret["Attributes"] = get_topic_attributes( arn, region=region, key=key, keyid=keyid, profile=profile ) + # Grab extended attributes for the above subscriptions + for sub in range(len(ret["Subscriptions"])): + sub_arn = ret["Subscriptions"][sub]["SubscriptionArn"] + if not sub_arn.startswith("arn:aws:sns:"): + # Sometimes a sub is in e.g. PendingAccept or other + # wierd states and doesn't have an ARN yet + log.debug("Subscription with invalid ARN %s skipped...", sub_arn) + continue return ret @@ -382,6 +390,17 @@ def unsubscribe(SubscriptionArn, region=None, key=None, keyid=None, profile=None salt myminion boto3_sns.unsubscribe my_subscription_arn region=us-east-1 """ + if not SubscriptionArn.startswith("arn:aws:sns:"): + # Grrr, AWS sent us an ARN that's NOT and ARN.... + # This can happen if, for instance, a subscription is left in PendingAcceptance or similar + # Note that anything left in PendingConfirmation will be auto-deleted by AWS after 30 days + # anyway, so this isn't as ugly a hack as it might seem at first... + log.info( + "Invalid subscription ARN `%s` passed - likely a PendingConfirmaton or such. " + "Skipping unsubscribe attempt as it would almost certainly fail...", + SubscriptionArn, + ) + return True subs = list_subscriptions(region=region, key=key, keyid=keyid, profile=profile) sub = [s for s in subs if s.get("SubscriptionArn") == SubscriptionArn] if not sub: diff --git a/salt/states/boto3_sns.py b/salt/states/boto3_sns.py index 375d666d6ca0..1acecfe9e7ed 100644 --- a/salt/states/boto3_sns.py +++ b/salt/states/boto3_sns.py @@ -233,7 +233,9 @@ def topic_present( subscribe += [sub] for sub in current_subs: minimal = {"Protocol": sub["Protocol"], "Endpoint": sub["Endpoint"]} - if minimal not in obfuscated_subs: + if minimal not in obfuscated_subs and sub["SubscriptionArn"].startswith( + "arn:aws:sns:" + ): unsubscribe += [sub["SubscriptionArn"]] for sub in subscribe: prot = sub["Protocol"] From c8f85ee751ad97fdea9d4a695a4fbaa95883fd52 Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 20 Apr 2020 11:15:34 -0600 Subject: [PATCH 09/33] Backport #53994 Fix typo perkissive to permissive --- tests/unit/daemons/test_masterapi.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/unit/daemons/test_masterapi.py b/tests/unit/daemons/test_masterapi.py index c787840f0f07..0be49a1f6bd3 100644 --- a/tests/unit/daemons/test_masterapi.py +++ b/tests/unit/daemons/test_masterapi.py @@ -105,7 +105,8 @@ def test_check_permissions_others_can_write(self): @patch_check_permissions() def test_check_permissions_group_can_write_not_permissive(self): """ - Assert that a file is accepted, when group can write to it and perkissive_pki_access=False + Assert that a file is accepted, when group can write to it and + permissive_pki_access=False """ self.stats["testfile"] = {"mode": gen_permissions("w", "w", ""), "gid": 1} if salt.utils.platform.is_windows(): @@ -116,7 +117,8 @@ def test_check_permissions_group_can_write_not_permissive(self): @patch_check_permissions(permissive_pki=True) def test_check_permissions_group_can_write_permissive(self): """ - Assert that a file is accepted, when group can write to it and perkissive_pki_access=True + Assert that a file is accepted, when group can write to it and + permissive_pki_access=True """ self.stats["testfile"] = {"mode": gen_permissions("w", "w", ""), "gid": 1} self.assertTrue(self.auto_key.check_permissions("testfile")) @@ -124,8 +126,8 @@ def test_check_permissions_group_can_write_permissive(self): @patch_check_permissions(uid=0, permissive_pki=True) def test_check_permissions_group_can_write_permissive_root_in_group(self): """ - Assert that a file is accepted, when group can write to it, perkissive_pki_access=False, - salt is root and in the file owning group + Assert that a file is accepted, when group can write to it, + permissive_pki_access=False, salt is root and in the file owning group """ self.stats["testfile"] = {"mode": gen_permissions("w", "w", ""), "gid": 0} self.assertTrue(self.auto_key.check_permissions("testfile")) @@ -133,8 +135,9 @@ def test_check_permissions_group_can_write_permissive_root_in_group(self): @patch_check_permissions(uid=0, permissive_pki=True) def test_check_permissions_group_can_write_permissive_root_not_in_group(self): """ - Assert that no file is accepted, when group can write to it, perkissive_pki_access=False, - salt is root and **not** in the file owning group + Assert that no file is accepted, when group can write to it, + permissive_pki_access=False, salt is root and **not** in the file owning + group """ self.stats["testfile"] = {"mode": gen_permissions("w", "w", ""), "gid": 1} if salt.utils.platform.is_windows(): From 591e8108a848ed7e4cb3b418fa4d33a4f3d0a8b0 Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 20 Apr 2020 11:18:46 -0600 Subject: [PATCH 10/33] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42b7ab0eb44d..69b0acdfb881 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Versions are `MAJOR.PATCH`. ### Deprecated ### Changed +- [#56731](https://github.com/saltstack/salt/pull/56731) - Backport #53994 ### Fixed - [#56237](https://github.com/saltstack/salt/pull/56237) - Fix alphabetical ordering and remove duplicates across all documentation indexes - [@myii](https://github.com/myii) From 32aeff6b6e33130f6670265361cedbe1b4d21926 Mon Sep 17 00:00:00 2001 From: Daniel Wozniak Date: Tue, 8 Jan 2019 11:12:05 -0700 Subject: [PATCH 11/33] Merge pull request #51095 from afischer-opentext-com/get_zone Avoid reading Windows registry value which may not be in place --- salt/modules/win_timezone.py | 22 +++--- tests/unit/modules/test_win_timezone.py | 101 ++++++++++++++++-------- 2 files changed, 79 insertions(+), 44 deletions(-) diff --git a/salt/modules/win_timezone.py b/salt/modules/win_timezone.py index 64c1134ef93a..b64aa05c5a17 100644 --- a/salt/modules/win_timezone.py +++ b/salt/modules/win_timezone.py @@ -209,24 +209,22 @@ def get_zone(): Returns: str: Timezone in unix format + Raises: + CommandExecutionError: If timezone could not be gathered + CLI Example: .. code-block:: bash salt '*' timezone.get_zone """ - win_zone = __utils__["reg.read_value"]( - hive="HKLM", - key="SYSTEM\\CurrentControlSet\\Control\\TimeZoneInformation", - vname="TimeZoneKeyName", - )["vdata"] - # Some data may have null characters. We only need the first portion up to - # the first null character. See the following: - # https://github.com/saltstack/salt/issues/51940 - # https://stackoverflow.com/questions/27716746/hklm-system-currentcontrolset-control-timezoneinformation-timezonekeyname-corrup - if "\0" in win_zone: - win_zone = win_zone.split("\0")[0] - return mapper.get_unix(win_zone.lower(), "Unknown") + cmd = ["tzutil", "/g"] + res = __salt__["cmd.run_all"](cmd, python_shell=False) + if res["retcode"] or not res["stdout"]: + raise CommandExecutionError( + "tzutil encountered an error getting timezone", info=res + ) + return mapper.get_unix(res["stdout"].lower(), "Unknown") def get_offset(): diff --git a/tests/unit/modules/test_win_timezone.py b/tests/unit/modules/test_win_timezone.py index dfea7b5084f8..8c1ae98fbe61 100644 --- a/tests/unit/modules/test_win_timezone.py +++ b/tests/unit/modules/test_win_timezone.py @@ -7,6 +7,7 @@ # Import Salt Libs import salt.modules.win_timezone as win_timezone +from salt.exceptions import CommandExecutionError # Import Salt Testing Libs from tests.support.mixins import LoaderModuleMockMixin @@ -23,47 +24,62 @@ class WinTimezoneTestCase(TestCase, LoaderModuleMockMixin): def setup_loader_modules(self): return {win_timezone: {}} - # 'get_zone' function tests: 3 - - def test_get_zone(self): + def test_get_zone_normal(self): """ - Test if it gets current timezone (i.e. Asia/Calcutta) + Test if it get current timezone (i.e. Asia/Calcutta) """ - mock_read = MagicMock( - side_effect=[ - {"vdata": "India Standard Time"}, - {"vdata": "Indian Standard Time"}, - ] + mock_read_ok = MagicMock( + return_value={ + "pid": 78, + "retcode": 0, + "stderr": "", + "stdout": "India Standard Time", + } ) - - with patch.dict(win_timezone.__utils__, {"reg.read_value": mock_read}): + with patch.dict(win_timezone.__salt__, {"cmd.run_all": mock_read_ok}): self.assertEqual(win_timezone.get_zone(), "Asia/Calcutta") - self.assertEqual(win_timezone.get_zone(), "Unknown") - def test_get_zone_null_terminated(self): + def test_get_zone_unknown(self): """ - Test if it handles instances where the registry contains null values + Test get_zone with unknown timezone (i.e. Indian Standard Time) """ - mock_read = MagicMock( - side_effect=[ - {"vdata": "India Standard Time\0\0\0\0"}, - {"vdata": "Indian Standard Time\0\0some more junk data\0\0"}, - ] + mock_read_error = MagicMock( + return_value={ + "pid": 78, + "retcode": 0, + "stderr": "", + "stdout": "Indian Standard Time", + } ) - - with patch.dict(win_timezone.__utils__, {"reg.read_value": mock_read}): - self.assertEqual(win_timezone.get_zone(), "Asia/Calcutta") + with patch.dict(win_timezone.__salt__, {"cmd.run_all": mock_read_error}): self.assertEqual(win_timezone.get_zone(), "Unknown") + def test_get_zone_error(self): + """ + Test get_zone when it encounters an error + """ + mock_read_fatal = MagicMock( + return_value={"pid": 78, "retcode": 1, "stderr": "", "stdout": ""} + ) + with patch.dict(win_timezone.__salt__, {"cmd.run_all": mock_read_fatal}): + self.assertRaises(CommandExecutionError, win_timezone.get_zone) + # 'get_offset' function tests: 1 def test_get_offset(self): """ Test if it get current numeric timezone offset from UCT (i.e. +0530) """ - mock_read = MagicMock(return_value={"vdata": "India Standard Time"}) + mock_read = MagicMock( + return_value={ + "pid": 78, + "retcode": 0, + "stderr": "", + "stdout": "India Standard Time", + } + ) - with patch.dict(win_timezone.__utils__, {"reg.read_value": mock_read}): + with patch.dict(win_timezone.__salt__, {"cmd.run_all": mock_read}): self.assertEqual(win_timezone.get_offset(), "+0530") # 'get_zonecode' function tests: 1 @@ -72,9 +88,16 @@ def test_get_zonecode(self): """ Test if it get current timezone (i.e. PST, MDT, etc) """ - mock_read = MagicMock(return_value={"vdata": "India Standard Time"}) + mock_read = MagicMock( + return_value={ + "pid": 78, + "retcode": 0, + "stderr": "", + "stdout": "India Standard Time", + } + ) - with patch.dict(win_timezone.__utils__, {"reg.read_value": mock_read}): + with patch.dict(win_timezone.__salt__, {"cmd.run_all": mock_read}): self.assertEqual(win_timezone.get_zonecode(), "IST") # 'set_zone' function tests: 1 @@ -83,13 +106,20 @@ def test_set_zone(self): """ Test if it unlinks, then symlinks /etc/localtime to the set timezone. """ - mock_cmd = MagicMock( + mock_write = MagicMock( return_value={"pid": 78, "retcode": 0, "stderr": "", "stdout": ""} ) - mock_read = MagicMock(return_value={"vdata": "India Standard Time"}) + mock_read = MagicMock( + return_value={ + "pid": 78, + "retcode": 0, + "stderr": "", + "stdout": "India Standard Time", + } + ) - with patch.dict(win_timezone.__salt__, {"cmd.run_all": mock_cmd}), patch.dict( - win_timezone.__utils__, {"reg.read_value": mock_read} + with patch.dict(win_timezone.__salt__, {"cmd.run_all": mock_write}), patch.dict( + win_timezone.__salt__, {"cmd.run_all": mock_read} ): self.assertTrue(win_timezone.set_zone("Asia/Calcutta")) @@ -102,9 +132,16 @@ def test_zone_compare(self): the one set in /etc/localtime. Returns True if they match, and False if not. Mostly useful for running state checks. """ - mock_read = MagicMock(return_value={"vdata": "India Standard Time"}) + mock_read = MagicMock( + return_value={ + "pid": 78, + "retcode": 0, + "stderr": "", + "stdout": "India Standard Time", + } + ) - with patch.dict(win_timezone.__utils__, {"reg.read_value": mock_read}): + with patch.dict(win_timezone.__salt__, {"cmd.run_all": mock_read}): self.assertTrue(win_timezone.zone_compare("Asia/Calcutta")) # 'get_hwclock' function tests: 1 From 60e87872f3c34ff87e9690f7f214b2b36fd5932d Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 20 Apr 2020 17:23:36 -0600 Subject: [PATCH 12/33] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0ad10ee89f8..af8b74ec17fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Versions are `MAJOR.PATCH`. ### Deprecated ### Changed +- [#56753](https://github.com/saltstack/salt/pull/56753) - Backport 51095 ### Fixed - [#56237](https://github.com/saltstack/salt/pull/56237) - Fix alphabetical ordering and remove duplicates across all documentation indexes - [@myii](https://github.com/myii) From e79fcecd54919aff49cc47c3bb41262b6c0a1626 Mon Sep 17 00:00:00 2001 From: Adam Mendlik Date: Thu, 18 Oct 2018 07:22:11 -0600 Subject: [PATCH 13/33] For the saltify cloud provider, use minion name as ssh_host --- salt/cloud/clouds/saltify.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/salt/cloud/clouds/saltify.py b/salt/cloud/clouds/saltify.py index d15c93b5c0e3..a65d0cecbeb3 100644 --- a/salt/cloud/clouds/saltify.py +++ b/salt/cloud/clouds/saltify.py @@ -268,6 +268,10 @@ def create(vm_): "deploy", vm_, __opts__, default=False ) + # If ssh_host is not set, default to the minion name + if not config.get_cloud_config_value("ssh_host", vm_, __opts__, default=""): + vm_["ssh_host"] = vm_["name"] + if deploy_config: wol_mac = config.get_cloud_config_value( "wake_on_lan_mac", vm_, __opts__, default="" From 353a06ff9581706a0dfeaf97510442705ace457e Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Mon, 20 Apr 2020 19:30:10 -0500 Subject: [PATCH 14/33] Add test case --- tests/unit/cloud/clouds/test_saltify.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/unit/cloud/clouds/test_saltify.py b/tests/unit/cloud/clouds/test_saltify.py index 83cb45adbc9f..21a5ca99027f 100644 --- a/tests/unit/cloud/clouds/test_saltify.py +++ b/tests/unit/cloud/clouds/test_saltify.py @@ -82,6 +82,31 @@ def test_create_and_deploy(self): mock_cmd.assert_called_once_with(vm_, ANY) self.assertTrue(result) + def test_create_no_ssh_host(self): + """ + Test that ssh_host is set to the vm name if not defined + """ + mock_cmd = MagicMock(return_value=True) + with patch.dict( + "salt.cloud.clouds.saltify.__utils__", {"cloud.bootstrap": mock_cmd} + ): + vm_ = { + "deploy": True, + "driver": "saltify", + "name": "new2", + "profile": "testprofile2", + } + result = saltify.create(vm_) + mock_cmd.assert_called_once_with(vm_, ANY) + assert result + # Make sure that ssh_host was added to the vm. Note that this is + # done in two asserts so that the failure is more explicit about + # what is wrong. If ssh_host wasn't inserted in the vm_ dict, the + # failure would be a KeyError, which would be harder to + # troubleshoot. + assert "ssh_host" in vm_ + assert vm_["ssh_host"] == "new2" + def test_create_wake_on_lan(self): """ Test if wake on lan works From 90c57cfae84ff246c7625a10b4942acc40316665 Mon Sep 17 00:00:00 2001 From: Adam Mendlik Date: Wed, 29 Aug 2018 06:17:34 -0600 Subject: [PATCH 15/33] Add banner function to modules.slsutil --- salt/modules/slsutil.py | 103 +++++++++++++++++++++++++++++ tests/unit/modules/test_slsutil.py | 53 +++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 tests/unit/modules/test_slsutil.py diff --git a/salt/modules/slsutil.py b/salt/modules/slsutil.py index d4c2ef2ff4f7..538491e9bb72 100644 --- a/salt/modules/slsutil.py +++ b/salt/modules/slsutil.py @@ -6,6 +6,8 @@ # Import Python libs from __future__ import absolute_import, print_function, unicode_literals +import textwrap + # Import Salt libs import salt.exceptions import salt.loader @@ -243,3 +245,104 @@ def deserialize(serializer, stream_or_string, **mod_kwargs): """ kwargs = salt.utils.args.clean_kwargs(**mod_kwargs) return _get_serialize_fn(serializer, "deserialize")(stream_or_string, **kwargs) + + +def banner( + width=72, + commentchar="#", + borderchar="#", + blockstart=None, + blockend=None, + title=None, + text=None, + newline=False, +): + """ + Create a standardized comment block to include in a templated file. + + A common technique in configuration management is to include a comment + block in managed files, warning users not to modify the file. This + function simplifies and standardizes those comment blocks. + + :param width: The width, in characters, of the banner. Default is 72. + :param commentchar: The character to be used in the starting position of + each line. This value should be set to a valid line comment character + for the syntax of the file in which the banner is being inserted. + Multiple character sequences, like '//' are supported. + If the file's syntax does not support line comments (such as XML), + use the ``blockstart`` and ``blockend`` options. + :param borderchar: The character to use in the top and bottom border of + the comment box. Must be a single character. + :param blockstart: The character sequence to use at the beginning of a + block comment. Should be used in conjunction with ``blockend`` + :param blockend: The character sequence to use at the end of a + block comment. Should be used in conjunction with ``blockstart`` + :param title: The first field of the comment block. This field appears + centered at the top of the box. + :param text: The second filed of the comment block. This field appears + left-justifed at the bottom of the box. + :param newline: Boolean value to indicate whether the comment block should + end with a newline. Default is ``False``. + + This banner can be injected into any templated file, for example: + + .. code-block:: jinja + + {{ salt['slsutil.banner'](width=120, commentchar='//') }} + + The default banner: + + .. code-block:: none + + ######################################################################## + # # + # THIS FILE IS MANAGED BY SALT - DO NOT EDIT # + # # + # The contents of this file are managed by Salt. Any changes to this # + # file may be overwritten automatically and without warning. # + ######################################################################## + """ + + if title is None: + title = "THIS FILE IS MANAGED BY SALT - DO NOT EDIT" + + if text is None: + text = ( + "The contents of this file are managed by Salt. " + "Any changes to this file may be overwritten " + "automatically and without warning" + ) + + # Set up some typesetting variables + lgutter = commentchar.strip() + " " + rgutter = " " + commentchar.strip() + textwidth = width - len(lgutter) - len(rgutter) + border_line = ( + commentchar + borderchar[:1] * (width - len(commentchar) * 2) + commentchar + ) + spacer_line = commentchar + " " * (width - len(commentchar) * 2) + commentchar + wrapper = textwrap.TextWrapper(width=(width - len(lgutter) - len(rgutter))) + block = list() + + # Create the banner + if blockstart is not None: + block.append(blockstart) + block.append(border_line) + block.append(spacer_line) + for line in wrapper.wrap(title): + block.append(lgutter + line.center(textwidth) + rgutter) + block.append(spacer_line) + for line in wrapper.wrap(text): + block.append(lgutter + line + " " * (textwidth - len(line)) + rgutter) + block.append(border_line) + if blockend is not None: + block.append(blockend) + + # Convert list to multiline string + result = "\n".join(block) + + # Add a newline character to the end of the banner + if newline: + return result + "\n" + + return result diff --git a/tests/unit/modules/test_slsutil.py b/tests/unit/modules/test_slsutil.py new file mode 100644 index 000000000000..aa6676fbba61 --- /dev/null +++ b/tests/unit/modules/test_slsutil.py @@ -0,0 +1,53 @@ +# -*- coding: utf-8 -*- + +from __future__ import absolute_import + +import logging + +import salt.modules.slsutil as slsutil +from tests.support.unit import TestCase + +log = logging.getLogger(__name__) + + +class SlsUtilTestCase(TestCase): + """ + Test cases for salt.modules.slsutil + """ + + def test_banner(self): + """ + Test banner function + """ + self.check_banner() + self.check_banner(width=81) + self.check_banner(width=20) + self.check_banner(commentchar="//", borderchar="-") + self.check_banner(title="title here", text="text here") + + def check_banner( + self, + width=72, + commentchar="#", + borderchar="#", + blockstart=None, + blockend=None, + title=None, + text=None, + newline=True, + ): + + result = slsutil.banner( + width=width, + commentchar=commentchar, + borderchar=borderchar, + blockstart=blockstart, + blockend=blockend, + title=title, + text=text, + newline=newline, + ).splitlines() + for line in result: + self.assertEqual(len(line), width) + self.assertTrue(line.startswith(commentchar)) + self.assertTrue(line.endswith(commentchar)) From 2ebd2ea75aabf27539f19a05df8abe7d06250eb3 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Wed, 17 Oct 2018 15:32:01 -0700 Subject: [PATCH 16/33] Copying changes from integration.sdb.test_vault over to integration.module.test_vault. --- tests/integration/modules/test_vault.py | 41 +++++++++++++++++++------ 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/tests/integration/modules/test_vault.py b/tests/integration/modules/test_vault.py index aef91ee63ffa..96020a6d7acc 100644 --- a/tests/integration/modules/test_vault.py +++ b/tests/integration/modules/test_vault.py @@ -3,20 +3,16 @@ Integration tests for the vault execution module """ -# Import Python Libs from __future__ import absolute_import, print_function, unicode_literals import inspect import logging import time -# Import Salt Libs import salt.utils.path from tests.support.case import ModuleCase from tests.support.helpers import destructiveTest from tests.support.paths import FILES - -# Import Salt Testing Libs from tests.support.unit import skipIf log = logging.getLogger(__name__) @@ -36,7 +32,7 @@ def setUp(self): """ SetUp vault container """ - if self.count == 0: + if VaultTestCase.count == 0: config = '{"backend": {"file": {"path": "/vault/file"}}, "default_lease_ttl": "168h", "max_lease_ttl": "720h", "disable_mlock": true}' self.run_state("docker_image.present", name="vault", tag="0.9.6") self.run_state( @@ -55,8 +51,31 @@ def setUp(self): cmd="/usr/local/bin/vault login token=testsecret", env={"VAULT_ADDR": "http://127.0.0.1:8200"}, ) - if ret != 0: - self.skipTest("unable to login to vault") + login_attempts = 1 + # If the login failed, container might have stopped + # attempt again, maximum of three times before + # skipping. + while ret != 0: + self.run_state( + "docker_container.running", + name="vault", + image="vault:0.9.6", + port_bindings="8200:8200", + environment={ + "VAULT_DEV_ROOT_TOKEN_ID": "testsecret", + "VAULT_LOCAL_CONFIG": config, + }, + cap_add="IPC_LOCK", + ) + time.sleep(5) + ret = self.run_function( + "cmd.retcode", + cmd="/usr/local/bin/vault login token=testsecret", + env={"VAULT_ADDR": "http://127.0.0.1:8200"}, + ) + login_attempts += 1 + if login_attempts >= 3: + self.skipTest("unable to login to vault") ret = self.run_function( "cmd.retcode", cmd="/usr/local/bin/vault policy write testpolicy {0}/vault.hcl".format( @@ -66,7 +85,7 @@ def setUp(self): ) if ret != 0: self.skipTest("unable to assign policy to vault") - self.count += 1 + VaultTestCase.count += 1 def tearDown(self): """ @@ -74,10 +93,12 @@ def tearDown(self): """ def count_tests(funcobj): - return inspect.ismethod(funcobj) and funcobj.__name__.startswith("test_") + return ( + inspect.ismethod(funcobj) or inspect.isfunction(funcobj) + ) and funcobj.__name__.startswith("test_") numtests = len(inspect.getmembers(VaultTestCase, predicate=count_tests)) - if self.count >= numtests: + if VaultTestCase.count >= numtests: self.run_state("docker_container.stopped", name="vault") self.run_state("docker_container.absent", name="vault") self.run_state("docker_image.absent", name="vault", force=True) From e948532a14bc18aa199100156c6640b5f50d5091 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Tue, 21 Apr 2020 11:20:38 +0100 Subject: [PATCH 17/33] Improve tests setup/teardown. Additionally, don't use a fixed path to the `vaul` binary --- tests/integration/modules/test_vault.py | 195 +++++++++++------------- tests/support/case.py | 8 + 2 files changed, 94 insertions(+), 109 deletions(-) diff --git a/tests/integration/modules/test_vault.py b/tests/integration/modules/test_vault.py index 96020a6d7acc..ce5504951037 100644 --- a/tests/integration/modules/test_vault.py +++ b/tests/integration/modules/test_vault.py @@ -5,38 +5,41 @@ from __future__ import absolute_import, print_function, unicode_literals -import inspect import logging import time import salt.utils.path from tests.support.case import ModuleCase from tests.support.helpers import destructiveTest -from tests.support.paths import FILES -from tests.support.unit import skipIf +from tests.support.runtests import RUNTIME_VARS +from tests.support.sminion import create_sminion +from tests.support.unit import SkipTest, skipIf log = logging.getLogger(__name__) +VAULT_BINARY_PATH = salt.utils.path.which("vault") + @destructiveTest @skipIf(not salt.utils.path.which("dockerd"), "Docker not installed") -@skipIf(not salt.utils.path.which("vault"), "Vault not installed") +@skipIf(not VAULT_BINARY_PATH, "Vault not installed") class VaultTestCase(ModuleCase): """ Test vault module """ - count = 0 - - def setUp(self): - """ - SetUp vault container - """ - if VaultTestCase.count == 0: - config = '{"backend": {"file": {"path": "/vault/file"}}, "default_lease_ttl": "168h", "max_lease_ttl": "720h", "disable_mlock": true}' - self.run_state("docker_image.present", name="vault", tag="0.9.6") - self.run_state( - "docker_container.running", + @classmethod + def setUpClass(cls): + cls.sminion = sminion = create_sminion() + config = '{"backend": {"file": {"path": "/vault/file"}}, "default_lease_ttl": "168h", "max_lease_ttl": "720h", "disable_mlock": true}' + sminion.states.docker_image.present(name="vault", tag="0.9.6") + login_attempts = 1 + container_created = False + while True: + if container_created: + sminion.states.docker_container.stopped(name="vault") + sminion.states.docker_container.absent(name="vault") + ret = sminion.states.docker_container.running( name="vault", image="vault:0.9.6", port_bindings="8200:8200", @@ -45,63 +48,37 @@ def setUp(self): "VAULT_LOCAL_CONFIG": config, }, ) + log.debug("docker_container.running return: %s", ret) + container_created = ret["result"] time.sleep(5) - ret = self.run_function( - "cmd.retcode", - cmd="/usr/local/bin/vault login token=testsecret", + ret = sminion.functions.cmd.run_all( + cmd="{} login token=testsecret".format(VAULT_BINARY_PATH), env={"VAULT_ADDR": "http://127.0.0.1:8200"}, + hide_output=False, ) - login_attempts = 1 - # If the login failed, container might have stopped - # attempt again, maximum of three times before - # skipping. - while ret != 0: - self.run_state( - "docker_container.running", - name="vault", - image="vault:0.9.6", - port_bindings="8200:8200", - environment={ - "VAULT_DEV_ROOT_TOKEN_ID": "testsecret", - "VAULT_LOCAL_CONFIG": config, - }, - cap_add="IPC_LOCK", - ) - time.sleep(5) - ret = self.run_function( - "cmd.retcode", - cmd="/usr/local/bin/vault login token=testsecret", - env={"VAULT_ADDR": "http://127.0.0.1:8200"}, - ) - login_attempts += 1 - if login_attempts >= 3: - self.skipTest("unable to login to vault") - ret = self.run_function( - "cmd.retcode", - cmd="/usr/local/bin/vault policy write testpolicy {0}/vault.hcl".format( - FILES - ), - env={"VAULT_ADDR": "http://127.0.0.1:8200"}, - ) - if ret != 0: - self.skipTest("unable to assign policy to vault") - VaultTestCase.count += 1 - - def tearDown(self): - """ - TearDown vault container - """ - - def count_tests(funcobj): - return ( - inspect.ismethod(funcobj) or inspect.isfunction(funcobj) - ) and funcobj.__name__.startswith("test_") - - numtests = len(inspect.getmembers(VaultTestCase, predicate=count_tests)) - if VaultTestCase.count >= numtests: - self.run_state("docker_container.stopped", name="vault") - self.run_state("docker_container.absent", name="vault") - self.run_state("docker_image.absent", name="vault", force=True) + if ret["retcode"] == 0: + break + log.debug("Vault login failed. Return: %s", ret) + login_attempts += 1 + + if login_attempts >= 3: + raise SkipTest("unable to login to vault") + + ret = sminion.functions.cmd.retcode( + cmd="{} policy write testpolicy {}/vault.hcl".format( + VAULT_BINARY_PATH, RUNTIME_VARS.FILES + ), + env={"VAULT_ADDR": "http://127.0.0.1:8200"}, + ) + if ret != 0: + raise SkipTest("unable to assign policy to vault") + + @classmethod + def tearDownClass(cls): + cls.sminion.states.docker_container.stopped(name="vault") + cls.sminion.states.docker_container.absent(name="vault") + cls.sminion.states.docker_image.absent(name="vault", force=True) + cls.sminion = None def test_write_read_secret(self): write_return = self.run_function( @@ -168,17 +145,18 @@ class VaultTestCaseCurrent(ModuleCase): Test vault module against current vault """ - count = 0 - - def setUp(self): - """ - SetUp vault container - """ - if self.count == 0: - config = '{"backend": {"file": {"path": "/vault/file"}}, "default_lease_ttl": "168h", "max_lease_ttl": "720h", "disable_mlock": true}' - self.run_state("docker_image.present", name="vault", tag="1.3.1") - self.run_state( - "docker_container.running", + @classmethod + def setUpClass(cls): + cls.sminion = sminion = create_sminion() + config = '{"backend": {"file": {"path": "/vault/file"}}, "default_lease_ttl": "168h", "max_lease_ttl": "720h", "disable_mlock": true}' + sminion.states.docker_image.present(name="vault", tag="1.3.1") + login_attempts = 1 + container_created = False + while True: + if container_created: + sminion.states.docker_container.stopped(name="vault") + sminion.states.docker_container.absent(name="vault") + ret = sminion.states.docker_container.running( name="vault", image="vault:1.3.1", port_bindings="8200:8200", @@ -187,38 +165,37 @@ def setUp(self): "VAULT_LOCAL_CONFIG": config, }, ) + log.debug("docker_container.running return: %s", ret) + container_created = ret["result"] time.sleep(5) - ret = self.run_function( - "cmd.retcode", - cmd="/usr/local/bin/vault login token=testsecret", - env={"VAULT_ADDR": "http://127.0.0.1:8200"}, - ) - if ret != 0: - self.skipTest("unable to login to vault") - ret = self.run_function( - "cmd.retcode", - cmd="/usr/local/bin/vault policy write testpolicy {0}/vault.hcl".format( - FILES - ), + ret = sminion.functions.cmd.run_all( + cmd="{} login token=testsecret".format(VAULT_BINARY_PATH), env={"VAULT_ADDR": "http://127.0.0.1:8200"}, + hide_output=False, ) - if ret != 0: - self.skipTest("unable to assign policy to vault") - self.count += 1 - - def tearDown(self): - """ - TearDown vault container - """ - - def count_tests(funcobj): - return inspect.ismethod(funcobj) and funcobj.__name__.startswith("test_") - - numtests = len(inspect.getmembers(VaultTestCaseCurrent, predicate=count_tests)) - if self.count >= numtests: - self.run_state("docker_container.stopped", name="vault") - self.run_state("docker_container.absent", name="vault") - self.run_state("docker_image.absent", name="vault", force=True) + if ret["retcode"] == 0: + break + log.debug("Vault login failed. Return: %s", ret) + login_attempts += 1 + + if login_attempts >= 3: + raise SkipTest("unable to login to vault") + + ret = sminion.functions.cmd.retcode( + cmd="{} policy write testpolicy {}/vault.hcl".format( + VAULT_BINARY_PATH, RUNTIME_VARS.FILES + ), + env={"VAULT_ADDR": "http://127.0.0.1:8200"}, + ) + if ret != 0: + raise SkipTest("unable to assign policy to vault") + + @classmethod + def tearDownClass(cls): + cls.sminion.states.docker_container.stopped(name="vault") + cls.sminion.states.docker_container.absent(name="vault") + cls.sminion.states.docker_image.absent(name="vault", force=True) + cls.sminion = None def test_write_read_secret_kv2(self): write_return = self.run_function( diff --git a/tests/support/case.py b/tests/support/case.py index 66686e606ee5..69098544b557 100644 --- a/tests/support/case.py +++ b/tests/support/case.py @@ -917,6 +917,14 @@ def run_function( if "f_timeout" in kwargs: kwargs["timeout"] = kwargs.pop("f_timeout") client = self.client if master_tgt is None else self.clients[master_tgt] + log.debug( + "Running client.cmd(minion_tgt=%r, function=%r, arg=%r, timeout=%r, kwarg=%r)", + minion_tgt, + function, + arg, + timeout, + kwargs, + ) orig = client.cmd(minion_tgt, function, arg, timeout=timeout, kwarg=kwargs) if RUNTIME_VARS.PYTEST_SESSION: From a087c1ef5cac5d5935d492fa8bdadb5bcf87db93 Mon Sep 17 00:00:00 2001 From: zer0def Date: Sun, 21 Oct 2018 21:29:28 +0200 Subject: [PATCH 18/33] Added `method_call` jinja filter. --- salt/utils/jinja.py | 5 +++++ tests/unit/utils/test_jinja.py | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/salt/utils/jinja.py b/salt/utils/jinja.py index 29b208568d14..378c600f03b1 100644 --- a/salt/utils/jinja.py +++ b/salt/utils/jinja.py @@ -670,6 +670,11 @@ def symmetric_difference(lst1, lst2): ) +@jinja_filter("method_call") +def method_call(obj, f_name, *f_args, **f_kwargs): + return getattr(obj, f_name, lambda *args, **kwargs: None)(*f_args, **f_kwargs) + + @jinja2.contextfunction def show_full_context(ctx): return salt.utils.data.simple_types_filter( diff --git a/tests/unit/utils/test_jinja.py b/tests/unit/utils/test_jinja.py index 8a6b57b18469..b9781cef765d 100644 --- a/tests/unit/utils/test_jinja.py +++ b/tests/unit/utils/test_jinja.py @@ -2,7 +2,6 @@ """ Tests for salt.utils.jinja """ -# Import Python libs from __future__ import absolute_import, print_function, unicode_literals import ast @@ -13,7 +12,6 @@ import re import tempfile -# Import Salt libs import salt.config import salt.loader @@ -39,12 +37,9 @@ from tests.support.case import ModuleCase from tests.support.helpers import flaky from tests.support.mock import MagicMock, Mock, patch - -# Import Salt Testing libs from tests.support.runtests import RUNTIME_VARS from tests.support.unit import TestCase, skipIf -# Import 3rd party libs try: import timelib # pylint: disable=W0611 From 2d518b92877e0e87a6b7667b47c8b3b03dcd9c28 Mon Sep 17 00:00:00 2001 From: zer0def Date: Mon, 22 Oct 2018 12:25:53 +0200 Subject: [PATCH 19/33] Added brief documentation and unit test. --- doc/topics/jinja/index.rst | 50 ++++++++++++++++++++++++++++++++++ tests/unit/utils/test_jinja.py | 27 ++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/doc/topics/jinja/index.rst b/doc/topics/jinja/index.rst index ac5ad7a5979d..0e7d1989f7c3 100644 --- a/doc/topics/jinja/index.rst +++ b/doc/topics/jinja/index.rst @@ -649,6 +649,56 @@ Returns: 1, 4 +.. jinja_ref:: method_call + +``method_call`` +--------------- + +.. versionadded:: develop + +Returns a result of object's method call. + +Example #1: + +.. code-block:: jinja + + {{ [1, 2, 1, 3, 4] | method_call('index', 1, 1, 3) }} + +Returns: + +.. code-block:: text + + 2 + +This filter can be used with the `map filter`_ to apply object methods without +using loop constructs or temporary variables. + +Example #2: + +.. code-block:: jinja + + {% set host_list = ['web01.example.com', 'db01.example.com'] %} + {% set host_list_split = [] %} + {% for item in host_list %} + {% do host_list_split.append(item.split('.', 1)) %} + {% endfor %} + {{ host_list_split }} + +Example #3: + +.. code-block:: jinja + + {{ host_list|map('method_call', 'split', '.', 1)|list }} + +Return of examples #2 and #3: + +.. code-block:: text + + [[web01, example.com], [db01, example.com]] + +.. _`map filter`: http://jinja.pocoo.org/docs/2.10/templates/#map + + .. jinja_ref:: is_sorted ``is_sorted`` diff --git a/tests/unit/utils/test_jinja.py b/tests/unit/utils/test_jinja.py index b9781cef765d..82ad50a6a4d7 100644 --- a/tests/unit/utils/test_jinja.py +++ b/tests/unit/utils/test_jinja.py @@ -1571,6 +1571,33 @@ def test_symmetric_difference(self): ) self.assertEqual(rendered, "1, 4") + def test_method_call(self): + ''' + Test the `method_call` Jinja filter. + ''' + rendered = render_jinja_tmpl("{{ 6|method_call('bit_length') }}", + dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + self.assertEqual(rendered, "3") + rendered = render_jinja_tmpl("{{ 6.7|method_call('is_integer') }}", + dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + self.assertEqual(rendered, "False") + rendered = render_jinja_tmpl("{{ 'absaltba'|method_call('strip', 'ab') }}", + dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + self.assertEqual(rendered, "salt") + rendered = render_jinja_tmpl("{{ [1, 2, 1, 3, 4]|method_call('index', 1, 1, 3) }}", + dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + self.assertEqual(rendered, "2") + + # have to use `dictsort` to keep test result deterministic + rendered = render_jinja_tmpl("{{ {}|method_call('fromkeys', ['a', 'b', 'c'], 0)|dictsort }}", + dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + self.assertEqual(rendered, "[('a', 0), ('b', 0), ('c', 0)]") + + # missing object method test + rendered = render_jinja_tmpl("{{ 6|method_call('bit_width') }}", + dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + self.assertEqual(rendered, "None") + def test_md5(self): """ Test the `md5` Jinja filter. From b3bdcb833942a0898031b8fd50a0b6978976a37e Mon Sep 17 00:00:00 2001 From: Mike Place Date: Tue, 23 Oct 2018 08:40:20 -0600 Subject: [PATCH 20/33] Change versionadded Per @rallytime review --- doc/topics/jinja/index.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/topics/jinja/index.rst b/doc/topics/jinja/index.rst index 0e7d1989f7c3..95fa2e2d755a 100644 --- a/doc/topics/jinja/index.rst +++ b/doc/topics/jinja/index.rst @@ -87,7 +87,7 @@ the context into the included file is required: .. code-block:: jinja {% from 'lib.sls' import test with context %} - + Includes must use full paths, like so: .. code-block:: jinja @@ -654,7 +654,7 @@ Returns: ``method_call`` --------------- -.. versionadded:: develop +.. versionadded:: Sodium Returns a result of object's method call. From bbc706a890885ca45c1c0834d458be96bfd74c90 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Tue, 21 Apr 2020 12:34:22 +0100 Subject: [PATCH 21/33] Remove Py2 specific tests. Improve test class setup/teardown. --- tests/unit/utils/test_jinja.py | 170 ++++++++++++++------------------- 1 file changed, 71 insertions(+), 99 deletions(-) diff --git a/tests/unit/utils/test_jinja.py b/tests/unit/utils/test_jinja.py index 82ad50a6a4d7..5bc0cff51536 100644 --- a/tests/unit/utils/test_jinja.py +++ b/tests/unit/utils/test_jinja.py @@ -122,6 +122,7 @@ def setUp(self): def tearDown(self): salt.utils.files.rm_rf(self.tempdir) + self.tempdir = self.template_dir = self.opts def test_searchpath(self): """ @@ -279,6 +280,7 @@ def setUp(self): def tearDown(self): salt.utils.files.rm_rf(self.tempdir) + self.tempdir = self.template_dir = self.local_opts = self.local_salt = None def test_fallback(self): """ @@ -554,19 +556,6 @@ def test_render_with_syntax_error(self): dict(opts=self.local_opts, saltenv="test", salt=self.local_salt), ) - @skipIf(six.PY3, "Not applicable to Python 3") - def test_render_with_unicode_syntax_error(self): - with patch.object(builtins, "__salt_system_encoding__", "utf-8"): - template = "hello\n\n{{ bad\n\nfoo한" - expected = r".*---\nhello\n\n{{ bad\n\nfoo\xed\x95\x9c <======================\n---" - self.assertRaisesRegex( - SaltRenderError, - expected, - render_jinja_tmpl, - template, - dict(opts=self.local_opts, saltenv="test", salt=self.local_salt), - ) - def test_render_with_utf8_syntax_error(self): with patch.object(builtins, "__salt_system_encoding__", "utf-8"): template = "hello\n\n{{ bad\n\nfoo한" @@ -616,9 +605,9 @@ def test_render_with_undefined_variable_unicode(self): class TestJinjaDefaultOptions(TestCase): - def __init__(self, *args, **kws): - TestCase.__init__(self, *args, **kws) - self.local_opts = { + @classmethod + def setUpClass(cls): + cls.local_opts = { "cachedir": os.path.join(RUNTIME_VARS.TMP, "jinja-template-cache"), "file_buffer_size": 1048576, "file_client": "local", @@ -637,11 +626,15 @@ def __init__(self, *args, **kws): ), "jinja_env": {"line_comment_prefix": "##", "line_statement_prefix": "%"}, } - self.local_salt = { + cls.local_salt = { "myvar": "zero", "mylist": [0, 1, 2, 3], } + @classmethod + def tearDownClass(cls): + cls.local_opts = cls.local_salt = None + def test_comment_prefix(self): template = """ @@ -676,9 +669,9 @@ def test_statement_prefix(self): class TestCustomExtensions(TestCase): - def __init__(self, *args, **kws): - super(TestCustomExtensions, self).__init__(*args, **kws) - self.local_opts = { + @classmethod + def setUpClass(cls): + cls.local_opts = { "cachedir": os.path.join(RUNTIME_VARS.TMP, "jinja-template-cache"), "file_buffer_size": 1048576, "file_client": "local", @@ -696,7 +689,7 @@ def __init__(self, *args, **kws): os.path.dirname(os.path.abspath(__file__)), "extmods" ), } - self.local_salt = { + cls.local_salt = { # 'dns.A': dnsutil.A, # 'dns.AAAA': dnsutil.AAAA, # 'file.exists': filemod.file_exists, @@ -704,6 +697,10 @@ def __init__(self, *args, **kws): # 'file.dirname': filemod.dirname } + @classmethod + def tearDownClass(cls): + cls.local_opts = cls.local_salt = None + def test_regex_escape(self): dataset = "foo?:.*/\\bar" env = Environment(extensions=[SerializerExtension]) @@ -716,51 +713,39 @@ def test_unique_string(self): unique = set(dataset) env = Environment(extensions=[SerializerExtension]) env.filters.update(JinjaFilter.salt_jinja_filters) - if six.PY3: - rendered = ( - env.from_string("{{ dataset|unique }}") - .render(dataset=dataset) - .strip("'{}") - .split("', '") - ) - self.assertEqual(sorted(rendered), sorted(list(unique))) - else: - rendered = env.from_string("{{ dataset|unique }}").render(dataset=dataset) - self.assertEqual(rendered, "{0}".format(unique)) + rendered = ( + env.from_string("{{ dataset|unique }}") + .render(dataset=dataset) + .strip("'{}") + .split("', '") + ) + self.assertEqual(sorted(rendered), sorted(list(unique))) def test_unique_tuple(self): dataset = ("foo", "foo", "bar") unique = set(dataset) env = Environment(extensions=[SerializerExtension]) env.filters.update(JinjaFilter.salt_jinja_filters) - if six.PY3: - rendered = ( - env.from_string("{{ dataset|unique }}") - .render(dataset=dataset) - .strip("'{}") - .split("', '") - ) - self.assertEqual(sorted(rendered), sorted(list(unique))) - else: - rendered = env.from_string("{{ dataset|unique }}").render(dataset=dataset) - self.assertEqual(rendered, "{0}".format(unique)) + rendered = ( + env.from_string("{{ dataset|unique }}") + .render(dataset=dataset) + .strip("'{}") + .split("', '") + ) + self.assertEqual(sorted(rendered), sorted(list(unique))) def test_unique_list(self): dataset = ["foo", "foo", "bar"] unique = ["foo", "bar"] env = Environment(extensions=[SerializerExtension]) env.filters.update(JinjaFilter.salt_jinja_filters) - if six.PY3: - rendered = ( - env.from_string("{{ dataset|unique }}") - .render(dataset=dataset) - .strip("'[]") - .split("', '") - ) - self.assertEqual(rendered, unique) - else: - rendered = env.from_string("{{ dataset|unique }}").render(dataset=dataset) - self.assertEqual(rendered, "{0}".format(unique)) + rendered = ( + env.from_string("{{ dataset|unique }}") + .render(dataset=dataset) + .strip("'[]") + .split("', '") + ) + self.assertEqual(rendered, unique) def test_serialize_json(self): dataset = {"foo": True, "bar": 42, "baz": [1, 2, 3], "qux": 2.0} @@ -790,17 +775,7 @@ def test_serialize_yaml_unicode(self): dataset = "str value" env = Environment(extensions=[SerializerExtension]) rendered = env.from_string("{{ dataset|yaml }}").render(dataset=dataset) - if six.PY3: - self.assertEqual("str value", rendered) - else: - # Due to a bug in the equality handler, this check needs to be split - # up into several different assertions. We need to check that the various - # string segments are present in the rendered value, as well as the - # type of the rendered variable (should be unicode, which is the same as - # six.text_type). This should cover all use cases but also allow the test - # to pass on CentOS 6 running Python 2.7. - self.assertIn("str value", rendered) - self.assertIsInstance(rendered, six.text_type) + self.assertEqual("str value", rendered) def test_serialize_python(self): dataset = {"foo": True, "bar": 42, "baz": [1, 2, 3], "qux": 2.0} @@ -971,20 +946,14 @@ def test_nested_structures(self): rendered = env.from_string("{{ data }}").render(data=data) self.assertEqual( - rendered, - "{u'foo': {u'bar': u'baz', u'qux': 42}}" - if six.PY2 - else "{'foo': {'bar': 'baz', 'qux': 42}}", + rendered, "{'foo': {'bar': 'baz', 'qux': 42}}", ) rendered = env.from_string("{{ data }}").render( data=[OrderedDict(foo="bar",), OrderedDict(baz=42,)] ) self.assertEqual( - rendered, - "[{'foo': u'bar'}, {'baz': 42}]" - if six.PY2 - else "[{'foo': 'bar'}, {'baz': 42}]", + rendered, "[{'foo': 'bar'}, {'baz': 42}]", ) def test_set_dict_key_value(self): @@ -1026,10 +995,7 @@ def test_update_dict_key_value(self): ), ) self.assertEqual( - rendered, - "{u'bar': {u'baz': {u'qux': 1, u'quux': 3}}}" - if six.PY2 - else "{'bar': {'baz': {'qux': 1, 'quux': 3}}}", + rendered, "{'bar': {'baz': {'qux': 1, 'quux': 3}}}", ) # Test incorrect usage @@ -1071,10 +1037,7 @@ def test_append_dict_key_value(self): ), ) self.assertEqual( - rendered, - "{u'bar': {u'baz': [1, 2, 42]}}" - if six.PY2 - else "{'bar': {'baz': [1, 2, 42]}}", + rendered, "{'bar': {'baz': [1, 2, 42]}}", ) def test_extend_dict_key_value(self): @@ -1097,10 +1060,7 @@ def test_extend_dict_key_value(self): ), ) self.assertEqual( - rendered, - "{u'bar': {u'baz': [1, 2, 42, 43]}}" - if six.PY2 - else "{'bar': {'baz': [1, 2, 42, 43]}}", + rendered, "{'bar': {'baz': [1, 2, 42, 43]}}", ) # Edge cases rendered = render_jinja_tmpl( @@ -1572,30 +1532,42 @@ def test_symmetric_difference(self): self.assertEqual(rendered, "1, 4") def test_method_call(self): - ''' + """ Test the `method_call` Jinja filter. - ''' - rendered = render_jinja_tmpl("{{ 6|method_call('bit_length') }}", - dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + """ + rendered = render_jinja_tmpl( + "{{ 6|method_call('bit_length') }}", + dict(opts=self.local_opts, saltenv="test", salt=self.local_salt), + ) self.assertEqual(rendered, "3") - rendered = render_jinja_tmpl("{{ 6.7|method_call('is_integer') }}", - dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + rendered = render_jinja_tmpl( + "{{ 6.7|method_call('is_integer') }}", + dict(opts=self.local_opts, saltenv="test", salt=self.local_salt), + ) self.assertEqual(rendered, "False") - rendered = render_jinja_tmpl("{{ 'absaltba'|method_call('strip', 'ab') }}", - dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + rendered = render_jinja_tmpl( + "{{ 'absaltba'|method_call('strip', 'ab') }}", + dict(opts=self.local_opts, saltenv="test", salt=self.local_salt), + ) self.assertEqual(rendered, "salt") - rendered = render_jinja_tmpl("{{ [1, 2, 1, 3, 4]|method_call('index', 1, 1, 3) }}", - dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + rendered = render_jinja_tmpl( + "{{ [1, 2, 1, 3, 4]|method_call('index', 1, 1, 3) }}", + dict(opts=self.local_opts, saltenv="test", salt=self.local_salt), + ) self.assertEqual(rendered, "2") # have to use `dictsort` to keep test result deterministic - rendered = render_jinja_tmpl("{{ {}|method_call('fromkeys', ['a', 'b', 'c'], 0)|dictsort }}", - dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + rendered = render_jinja_tmpl( + "{{ {}|method_call('fromkeys', ['a', 'b', 'c'], 0)|dictsort }}", + dict(opts=self.local_opts, saltenv="test", salt=self.local_salt), + ) self.assertEqual(rendered, "[('a', 0), ('b', 0), ('c', 0)]") # missing object method test - rendered = render_jinja_tmpl("{{ 6|method_call('bit_width') }}", - dict(opts=self.local_opts, saltenv='test', salt=self.local_salt)) + rendered = render_jinja_tmpl( + "{{ 6|method_call('bit_width') }}", + dict(opts=self.local_opts, saltenv="test", salt=self.local_salt), + ) self.assertEqual(rendered, "None") def test_md5(self): From 6690c9cfd0adbb7ac14f01245cfb530ef0944272 Mon Sep 17 00:00:00 2001 From: JD Robertson Date: Thu, 8 Nov 2018 14:41:01 -0600 Subject: [PATCH 22/33] Parse the command line from noaction operations and return the packages which would be modified Signed-off-by: JD Robertson Review cleanup Signed-off-by: JD Robertson More refactoring from review & fix pylint issues. Signed-off-by: JD Robertson Fix comment. Signed-off-by: JD Robertson Cleanup based on feedback Signed-off-by: JD Robertson --- salt/modules/opkg.py | 68 ++++++++++++++++++++++++++++++++- tests/unit/modules/test_opkg.py | 23 +++++------ 2 files changed, 77 insertions(+), 14 deletions(-) diff --git a/salt/modules/opkg.py b/salt/modules/opkg.py index b8c3f76bd42f..2341e22b0064 100644 --- a/salt/modules/opkg.py +++ b/salt/modules/opkg.py @@ -289,14 +289,48 @@ def refresh_db(failhard=False, **kwargs): # pylint: disable=unused-argument return ret +def _is_testmode(**kwargs): + """ + Returns whether a test mode (noaction) operation was requested. + """ + return bool(kwargs.get("test") or __opts__.get("test")) + + def _append_noaction_if_testmode(cmd, **kwargs): """ Adds the --noaction flag to the command if it's running in the test mode. """ - if bool(kwargs.get("test") or __opts__.get("test")): + if _is_testmode(**kwargs): cmd.append("--noaction") +def _parse_reported_packages_from_noaction_install_output(output): + """ + Parses the output of "opkg install" to determine what packages would have been + installed by an operation run with the --noaction flag. + + We are looking for lines like: + Installing () on + or + Upgrading from to on root + """ + reportedPkgs = {} + installPattern = re.compile( + r"Installing\s(?P.*?)\s\((?P.*?)\)\son\s(?P.*?)" + ) + upgradePattern = re.compile( + r"Upgrading\s(?P.*?)\sfrom\s(?P.*?)\sto\s(?P.*?)\son\s(?P.*?)" + ) + for line in salt.utils.itertools.split(output, "\n"): + match = installPattern.match(line) + if match is None: + match = upgradePattern.match(line) + if match: + reportedPkgs[match.group("package")] = match.group("version") + + return reportedPkgs + + def install( name=None, refresh=False, pkgs=None, sources=None, reinstall=False, **kwargs ): @@ -476,6 +510,13 @@ def install( __context__.pop("pkg.list_pkgs", None) new = list_pkgs() + if _is_testmode(**kwargs): + reportedPkgs = _parse_reported_packages_from_noaction_install_output( + out["stdout"] + ) + new = copy.deepcopy(new) + new.update(reportedPkgs) + ret = salt.utils.data.compare_dicts(old, new) if pkg_type == "file" and reinstall: @@ -513,6 +554,26 @@ def install( return ret +def _parse_reported_packages_from_noaction_remove_output(output): + """ + Parses the output of "opkg remove" to determine what packages would have been + removed by an operation run with the --noaction flag. + + We are looking for lines like + Removing () from ... + """ + reportedPkgs = {} + removePattern = re.compile( + r"Removing\s(?P.*?)\s\((?P.*?)\)\sfrom\s(?P.*?)..." + ) + for line in salt.utils.itertools.split(output, "\n"): + match = removePattern.match(line) + if match: + reportedPkgs[match.group("package")] = "" + + return reportedPkgs + + def remove(name=None, pkgs=None, **kwargs): # pylint: disable=unused-argument """ Remove packages using ``opkg remove``. @@ -576,6 +637,11 @@ def remove(name=None, pkgs=None, **kwargs): # pylint: disable=unused-argument __context__.pop("pkg.list_pkgs", None) new = list_pkgs() + if _is_testmode(**kwargs): + reportedPkgs = _parse_reported_packages_from_noaction_remove_output( + out["stdout"] + ) + new = {k: v for k, v in new.items() if k not in reportedPkgs} ret = salt.utils.data.compare_dicts(old, new) rs_result = _get_restartcheck_result(errors) diff --git a/tests/unit/modules/test_opkg.py b/tests/unit/modules/test_opkg.py index b295abfe061c..52ca43ae6f2e 100644 --- a/tests/unit/modules/test_opkg.py +++ b/tests/unit/modules/test_opkg.py @@ -3,24 +3,17 @@ :synopsis: Unit Tests for Package Management module 'module.opkg' :platform: Linux """ -# pylint: disable=import-error,3rd-party-module-not-gated -# Import Python libs from __future__ import absolute_import, print_function, unicode_literals import collections import copy import salt.modules.opkg as opkg - -# Import Salt Libs from salt.ext import six - -# Import Salt Testing Libs from tests.support.mixins import LoaderModuleMockMixin from tests.support.mock import MagicMock, patch from tests.support.unit import TestCase -# pylint: disable=import-error,3rd-party-module-not-gated OPKG_VIM_INFO = { "vim": { "Package": "vim", @@ -138,8 +131,9 @@ def test_install_noaction(self): """ Test - Install packages. """ - with patch("salt.modules.opkg.list_pkgs", MagicMock(return_value=({}))): - ret_value = {"retcode": 0} + with patch("salt.modules.opkg.list_pkgs", MagicMock(side_effect=({}, {}))): + std_out = "Downloading http://feedserver/feeds/test/vim_7.4_arch.ipk.\n\nInstalling vim (7.4) on root\n" + ret_value = {"retcode": 0, "stdout": std_out} mock = MagicMock(return_value=ret_value) patch_kwargs = { "__salt__": { @@ -153,7 +147,7 @@ def test_install_noaction(self): } } with patch.multiple(opkg, **patch_kwargs): - self.assertEqual(opkg.install("vim:7.4", test=True), {}) + self.assertEqual(opkg.install("vim:7.4", test=True), INSTALLED) def test_remove(self): """ @@ -182,8 +176,11 @@ def test_remove_noaction(self): """ Test - Remove packages. """ - with patch("salt.modules.opkg.list_pkgs", MagicMock(return_value=({}))): - ret_value = {"retcode": 0} + with patch( + "salt.modules.opkg.list_pkgs", MagicMock(side_effect=[PACKAGES, PACKAGES]) + ): + std_out = "\nRemoving vim (7.4) from root...\n" + ret_value = {"retcode": 0, "stdout": std_out} mock = MagicMock(return_value=ret_value) patch_kwargs = { "__salt__": { @@ -197,7 +194,7 @@ def test_remove_noaction(self): } } with patch.multiple(opkg, **patch_kwargs): - self.assertEqual(opkg.remove("vim:7.4", test=True), {}) + self.assertEqual(opkg.remove("vim:7.4", test=True), REMOVED) def test_info_installed(self): """ From 4bdb7125a2b9b272d7ae31ecd4bb3790801c3915 Mon Sep 17 00:00:00 2001 From: JD Robertson Date: Wed, 14 Nov 2018 16:02:00 -0600 Subject: [PATCH 23/33] Refactor for method length and fix multi-command cases Signed-off-by: JD Robertson --- salt/modules/opkg.py | 89 +++++++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 35 deletions(-) diff --git a/salt/modules/opkg.py b/salt/modules/opkg.py index 2341e22b0064..2340e0df869e 100644 --- a/salt/modules/opkg.py +++ b/salt/modules/opkg.py @@ -304,7 +304,31 @@ def _append_noaction_if_testmode(cmd, **kwargs): cmd.append("--noaction") -def _parse_reported_packages_from_noaction_install_output(output): +def _build_install_command_list(cmd_prefix, to_install, to_downgrade, to_reinstall): + """ + Builds a list of install commands to be executed in sequence in order to process + each of the to_install, to_downgrade, and to_reinstall lists. + """ + cmds = [] + if to_install: + cmd = copy.deepcopy(cmd_prefix) + cmd.extend(to_install) + cmds.append(cmd) + if to_downgrade: + cmd = copy.deepcopy(cmd_prefix) + cmd.append("--force-downgrade") + cmd.extend(to_downgrade) + cmds.append(cmd) + if to_reinstall: + cmd = copy.deepcopy(cmd_prefix) + cmd.append("--force-reinstall") + cmd.extend(to_reinstall) + cmds.append(cmd) + + return cmds + + +def _parse_reported_packages_from_install_output(output): """ Parses the output of "opkg install" to determine what packages would have been installed by an operation run with the --noaction flag. @@ -331,6 +355,25 @@ def _parse_reported_packages_from_noaction_install_output(output): return reportedPkgs +def _execute_install_commands(cmds, parse_output, errors, parsed_packages): + """ + Executes a sequence of install commands. + If any command fails, the command output will be appended to the errors list. + If parse_output is true, updated packages will be appended to the parsed_packages dictionary. + """ + for cmd in cmds: + out = __salt__["cmd.run_all"](cmd, output_loglevel="trace", python_shell=False) + if out["retcode"] != 0: + if out["stderr"]: + errors.append(out["stderr"]) + else: + errors.append(out["stdout"]) + elif parse_output: + parsed_packages.update( + _parse_reported_packages_from_install_output(out["stdout"]) + ) + + def install( name=None, refresh=False, pkgs=None, sources=None, reinstall=False, **kwargs ): @@ -474,24 +517,9 @@ def install( # This should cause the command to fail. to_install.append(pkgstr) - cmds = [] - - if to_install: - cmd = copy.deepcopy(cmd_prefix) - cmd.extend(to_install) - cmds.append(cmd) - - if to_downgrade: - cmd = copy.deepcopy(cmd_prefix) - cmd.append("--force-downgrade") - cmd.extend(to_downgrade) - cmds.append(cmd) - - if to_reinstall: - cmd = copy.deepcopy(cmd_prefix) - cmd.append("--force-reinstall") - cmd.extend(to_reinstall) - cmds.append(cmd) + cmds = _build_install_command_list( + cmd_prefix, to_install, to_downgrade, to_reinstall + ) if not cmds: return {} @@ -500,22 +528,15 @@ def install( refresh_db() errors = [] - for cmd in cmds: - out = __salt__["cmd.run_all"](cmd, output_loglevel="trace", python_shell=False) - if out["retcode"] != 0: - if out["stderr"]: - errors.append(out["stderr"]) - else: - errors.append(out["stdout"]) + is_testmode = _is_testmode(**kwargs) + test_packages = {} + _execute_install_commands(cmds, is_testmode, errors, test_packages) __context__.pop("pkg.list_pkgs", None) new = list_pkgs() - if _is_testmode(**kwargs): - reportedPkgs = _parse_reported_packages_from_noaction_install_output( - out["stdout"] - ) + if is_testmode: new = copy.deepcopy(new) - new.update(reportedPkgs) + new.update(test_packages) ret = salt.utils.data.compare_dicts(old, new) @@ -554,7 +575,7 @@ def install( return ret -def _parse_reported_packages_from_noaction_remove_output(output): +def _parse_reported_packages_from_remove_output(output): """ Parses the output of "opkg remove" to determine what packages would have been removed by an operation run with the --noaction flag. @@ -638,9 +659,7 @@ def remove(name=None, pkgs=None, **kwargs): # pylint: disable=unused-argument __context__.pop("pkg.list_pkgs", None) new = list_pkgs() if _is_testmode(**kwargs): - reportedPkgs = _parse_reported_packages_from_noaction_remove_output( - out["stdout"] - ) + reportedPkgs = _parse_reported_packages_from_remove_output(out["stdout"]) new = {k: v for k, v in new.items() if k not in reportedPkgs} ret = salt.utils.data.compare_dicts(old, new) From 9184c885e6b510138b045c22938b306c8a490100 Mon Sep 17 00:00:00 2001 From: JD Robertson Date: Wed, 14 Nov 2018 16:18:58 -0600 Subject: [PATCH 24/33] Cleanup variable names. Signed-off-by: JD Robertson --- salt/modules/opkg.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/salt/modules/opkg.py b/salt/modules/opkg.py index 2340e0df869e..6ed31c35c1c4 100644 --- a/salt/modules/opkg.py +++ b/salt/modules/opkg.py @@ -338,21 +338,21 @@ def _parse_reported_packages_from_install_output(output): or Upgrading from to on root """ - reportedPkgs = {} - installPattern = re.compile( + reported_pkgs = {} + install_pattern = re.compile( r"Installing\s(?P.*?)\s\((?P.*?)\)\son\s(?P.*?)" ) - upgradePattern = re.compile( + upgrade_pattern = re.compile( r"Upgrading\s(?P.*?)\sfrom\s(?P.*?)\sto\s(?P.*?)\son\s(?P.*?)" ) for line in salt.utils.itertools.split(output, "\n"): - match = installPattern.match(line) + match = install_pattern.match(line) if match is None: - match = upgradePattern.match(line) + match = upgrade_pattern.match(line) if match: - reportedPkgs[match.group("package")] = match.group("version") + reported_pkgs[match.group("package")] = match.group("version") - return reportedPkgs + return reported_pkgs def _execute_install_commands(cmds, parse_output, errors, parsed_packages): @@ -583,16 +583,16 @@ def _parse_reported_packages_from_remove_output(output): We are looking for lines like Removing () from ... """ - reportedPkgs = {} - removePattern = re.compile( + reported_pkgs = {} + remove_pattern = re.compile( r"Removing\s(?P.*?)\s\((?P.*?)\)\sfrom\s(?P.*?)..." ) for line in salt.utils.itertools.split(output, "\n"): - match = removePattern.match(line) + match = remove_pattern.match(line) if match: - reportedPkgs[match.group("package")] = "" + reported_pkgs[match.group("package")] = "" - return reportedPkgs + return reported_pkgs def remove(name=None, pkgs=None, **kwargs): # pylint: disable=unused-argument From 06698f0263859cb2696f568f5fc59fe773733b59 Mon Sep 17 00:00:00 2001 From: JD Robertson Date: Thu, 15 Nov 2018 08:43:15 -0600 Subject: [PATCH 25/33] Update refactor to reduce cognitive complexity. Signed-off-by: JD Robertson --- salt/modules/opkg.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/salt/modules/opkg.py b/salt/modules/opkg.py index 6ed31c35c1c4..d3202c74adc9 100644 --- a/salt/modules/opkg.py +++ b/salt/modules/opkg.py @@ -355,23 +355,23 @@ def _parse_reported_packages_from_install_output(output): return reported_pkgs -def _execute_install_commands(cmds, parse_output, errors, parsed_packages): +def _execute_install_command(cmd, parse_output, errors, parsed_packages): """ - Executes a sequence of install commands. - If any command fails, the command output will be appended to the errors list. - If parse_output is true, updated packages will be appended to the parsed_packages dictionary. + Executes a command for the install operation. + If the command fails, its error output will be appended to the errors list. + If the command succeeds and parse_output is true, updated packages will be appended + to the parsed_packages dictionary. """ - for cmd in cmds: - out = __salt__["cmd.run_all"](cmd, output_loglevel="trace", python_shell=False) - if out["retcode"] != 0: - if out["stderr"]: - errors.append(out["stderr"]) - else: - errors.append(out["stdout"]) - elif parse_output: - parsed_packages.update( - _parse_reported_packages_from_install_output(out["stdout"]) - ) + out = __salt__["cmd.run_all"](cmd, output_loglevel="trace", python_shell=False) + if out["retcode"] != 0: + if out["stderr"]: + errors.append(out["stderr"]) + else: + errors.append(out["stdout"]) + elif parse_output: + parsed_packages.update( + _parse_reported_packages_from_install_output(out["stdout"]) + ) def install( @@ -530,7 +530,8 @@ def install( errors = [] is_testmode = _is_testmode(**kwargs) test_packages = {} - _execute_install_commands(cmds, is_testmode, errors, test_packages) + for cmd in cmds: + _execute_install_command(cmd, is_testmode, errors, test_packages) __context__.pop("pkg.list_pkgs", None) new = list_pkgs() From f6650e8fab6115d92c83c70525943d7d2c528a64 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Tue, 21 Apr 2020 13:26:17 +0100 Subject: [PATCH 26/33] No test module global variables. --- tests/unit/modules/test_opkg.py | 95 ++++++++++++++++++--------------- 1 file changed, 51 insertions(+), 44 deletions(-) diff --git a/tests/unit/modules/test_opkg.py b/tests/unit/modules/test_opkg.py index 52ca43ae6f2e..509690e74df5 100644 --- a/tests/unit/modules/test_opkg.py +++ b/tests/unit/modules/test_opkg.py @@ -9,45 +9,49 @@ import copy import salt.modules.opkg as opkg -from salt.ext import six from tests.support.mixins import LoaderModuleMockMixin from tests.support.mock import MagicMock, patch from tests.support.unit import TestCase -OPKG_VIM_INFO = { - "vim": { - "Package": "vim", - "Version": "7.4.769-r0.31", - "Status": "install ok installed", - } -} - -OPKG_VIM_FILES = { - "errors": [], - "packages": { - "vim": [ - "/usr/bin/view", - "/usr/bin/vim.vim", - "/usr/bin/xxd", - "/usr/bin/vimdiff", - "/usr/bin/rview", - "/usr/bin/rvim", - "/usr/bin/ex", - ] - }, -} - -INSTALLED = {"vim": {"new": "7.4", "old": six.text_type()}} - -REMOVED = {"vim": {"new": six.text_type(), "old": "7.4"}} -PACKAGES = {"vim": "7.4"} - class OpkgTestCase(TestCase, LoaderModuleMockMixin): """ Test cases for salt.modules.opkg """ + @classmethod + def setUpClass(cls): + cls.opkg_vim_info = { + "vim": { + "Package": "vim", + "Version": "7.4.769-r0.31", + "Status": "install ok installed", + } + } + cls.opkg_vim_files = { + "errors": [], + "packages": { + "vim": [ + "/usr/bin/view", + "/usr/bin/vim.vim", + "/usr/bin/xxd", + "/usr/bin/vimdiff", + "/usr/bin/rview", + "/usr/bin/rvim", + "/usr/bin/ex", + ] + }, + } + cls.installed = {"vim": {"new": "7.4", "old": ""}} + cls.removed = {"vim": {"new": "", "old": "7.4"}} + cls.packages = {"vim": "7.4"} + + @classmethod + def tearDownClass(cls): + cls.opkg_vim_info = ( + cls.opkg_vim_files + ) = cls.installed = cls.removed = cls.packages = None + def setup_loader_modules(self): # pylint: disable=no-self-use """ Tested modules @@ -59,7 +63,7 @@ def test_version(self): Test - Returns a string representing the package version or an empty string if not installed. """ - version = OPKG_VIM_INFO["vim"]["Version"] + version = self.opkg_vim_info["vim"]["Version"] mock = MagicMock(return_value=version) with patch.dict(opkg.__salt__, {"pkg_resource.version": mock}): self.assertEqual(opkg.version(*["vim"]), version) @@ -75,22 +79,22 @@ def test_file_dict(self): """ Test - List the files that belong to a package, grouped by package. """ - std_out = "\n".join(OPKG_VIM_FILES["packages"]["vim"]) + std_out = "\n".join(self.opkg_vim_files["packages"]["vim"]) ret_value = {"stdout": std_out} mock = MagicMock(return_value=ret_value) with patch.dict(opkg.__salt__, {"cmd.run_all": mock}): - self.assertEqual(opkg.file_dict("vim"), OPKG_VIM_FILES) + self.assertEqual(opkg.file_dict("vim"), self.opkg_vim_files) def test_file_list(self): """ Test - List the files that belong to a package. """ - std_out = "\n".join(OPKG_VIM_FILES["packages"]["vim"]) + std_out = "\n".join(self.opkg_vim_files["packages"]["vim"]) ret_value = {"stdout": std_out} mock = MagicMock(return_value=ret_value) files = { - "errors": OPKG_VIM_FILES["errors"], - "files": OPKG_VIM_FILES["packages"]["vim"], + "errors": self.opkg_vim_files["errors"], + "files": self.opkg_vim_files["packages"]["vim"], } with patch.dict(opkg.__salt__, {"cmd.run_all": mock}): self.assertEqual(opkg.file_list("vim"), files) @@ -109,7 +113,7 @@ def test_install(self): Test - Install packages. """ with patch( - "salt.modules.opkg.list_pkgs", MagicMock(side_effect=[{}, PACKAGES]) + "salt.modules.opkg.list_pkgs", MagicMock(side_effect=[{}, self.packages]) ): ret_value = {"retcode": 0} mock = MagicMock(return_value=ret_value) @@ -125,7 +129,7 @@ def test_install(self): } } with patch.multiple(opkg, **patch_kwargs): - self.assertEqual(opkg.install("vim:7.4"), INSTALLED) + self.assertEqual(opkg.install("vim:7.4"), self.installed) def test_install_noaction(self): """ @@ -147,14 +151,14 @@ def test_install_noaction(self): } } with patch.multiple(opkg, **patch_kwargs): - self.assertEqual(opkg.install("vim:7.4", test=True), INSTALLED) + self.assertEqual(opkg.install("vim:7.4", test=True), self.installed) def test_remove(self): """ Test - Remove packages. """ with patch( - "salt.modules.opkg.list_pkgs", MagicMock(side_effect=[PACKAGES, {}]) + "salt.modules.opkg.list_pkgs", MagicMock(side_effect=[self.packages, {}]) ): ret_value = {"retcode": 0} mock = MagicMock(return_value=ret_value) @@ -170,14 +174,15 @@ def test_remove(self): } } with patch.multiple(opkg, **patch_kwargs): - self.assertEqual(opkg.remove("vim"), REMOVED) + self.assertEqual(opkg.remove("vim"), self.removed) def test_remove_noaction(self): """ Test - Remove packages. """ with patch( - "salt.modules.opkg.list_pkgs", MagicMock(side_effect=[PACKAGES, PACKAGES]) + "salt.modules.opkg.list_pkgs", + MagicMock(side_effect=[self.packages, self.packages]), ): std_out = "\nRemoving vim (7.4) from root...\n" ret_value = {"retcode": 0, "stdout": std_out} @@ -194,17 +199,19 @@ def test_remove_noaction(self): } } with patch.multiple(opkg, **patch_kwargs): - self.assertEqual(opkg.remove("vim:7.4", test=True), REMOVED) + self.assertEqual(opkg.remove("vim:7.4", test=True), self.removed) def test_info_installed(self): """ Test - Return the information of the named package(s) installed on the system. """ - installed = copy.deepcopy(OPKG_VIM_INFO["vim"]) + installed = copy.deepcopy(self.opkg_vim_info["vim"]) del installed["Package"] ordered_info = collections.OrderedDict(sorted(installed.items())) expected_dict = {"vim": {k.lower(): v for k, v in ordered_info.items()}} - std_out = "\n".join([k + ": " + v for k, v in OPKG_VIM_INFO["vim"].items()]) + std_out = "\n".join( + [k + ": " + v for k, v in self.opkg_vim_info["vim"].items()] + ) ret_value = {"stdout": std_out, "retcode": 0} mock = MagicMock(return_value=ret_value) with patch.dict(opkg.__salt__, {"cmd.run_all": mock}): From 86d282f89b941410266b412c4be2e5ecbeab7f5f Mon Sep 17 00:00:00 2001 From: Adam Mendlik Date: Wed, 29 Aug 2018 06:18:32 -0600 Subject: [PATCH 27/33] Add boolstr function to modules.slsutil --- salt/modules/slsutil.py | 38 ++++++++++++++++++++++++++++++ tests/unit/modules/test_slsutil.py | 7 ++++++ 2 files changed, 45 insertions(+) diff --git a/salt/modules/slsutil.py b/salt/modules/slsutil.py index 538491e9bb72..a4fd3bb05fa6 100644 --- a/salt/modules/slsutil.py +++ b/salt/modules/slsutil.py @@ -346,3 +346,41 @@ def banner( return result + "\n" return result + + +def boolstr(value, true='true', false='false'): + ''' + Convert a boolean value into a string. This function is + intended to be used from within file templates to provide + an easy way to take boolean values stored in Pillars or + Grains, and write them out in the apprpriate syntax for + a particular file template. + + :param value: The boolean value to be converted + :param true: The value to return if ``value`` is ``True`` + :param false: The value to return if ``value`` is ``False`` + + In this example, a pillar named ``smtp:encrypted`` stores a boolean + value, but the template that uses that value needs ``yes`` or ``no`` + to be written, based on the boolean value. + + *Note: this is written on two lines for clarity. The same result + could be achieved in one line.* + + .. code-block:: jinja + + {% set encrypted = salt[pillar.get]('smtp:encrypted', false) %} + use_tls: {{ salt['slsutil.boolstr'](encrypted, 'yes', 'no') }} + + Result (assuming the value is ``True``): + + .. code-block:: none + + use_tls: yes + + ''' + + if value: + return true + + return false diff --git a/tests/unit/modules/test_slsutil.py b/tests/unit/modules/test_slsutil.py index aa6676fbba61..48a84f75730c 100644 --- a/tests/unit/modules/test_slsutil.py +++ b/tests/unit/modules/test_slsutil.py @@ -51,3 +51,10 @@ def check_banner( self.assertEqual(len(line), width) self.assertTrue(line.startswith(commentchar)) self.assertTrue(line.endswith(commentchar)) + + def test_boolstr(self): + """ + Test boolstr function + """ + self.assertEqual("yes", slsutil.boolstr(True, true="yes", false="no")) + self.assertEqual("no", slsutil.boolstr(False, true="yes", false="no")) From acdd3f4512a26c869ab07d5399fba15594903461 Mon Sep 17 00:00:00 2001 From: Adam Mendlik Date: Fri, 31 Aug 2018 15:21:05 -0600 Subject: [PATCH 28/33] Use os.linesep for cross-platform compatibility --- salt/modules/slsutil.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/salt/modules/slsutil.py b/salt/modules/slsutil.py index a4fd3bb05fa6..1433e4eedc1c 100644 --- a/salt/modules/slsutil.py +++ b/salt/modules/slsutil.py @@ -6,6 +6,7 @@ # Import Python libs from __future__ import absolute_import, print_function, unicode_literals +import os import textwrap # Import Salt libs @@ -338,18 +339,18 @@ def banner( if blockend is not None: block.append(blockend) - # Convert list to multiline string - result = "\n".join(block) + # Convert list to multi-line string + result = os.linesep.join(block) # Add a newline character to the end of the banner if newline: - return result + "\n" + return result + os.linesep return result -def boolstr(value, true='true', false='false'): - ''' +def boolstr(value, true="true", false="false"): + """ Convert a boolean value into a string. This function is intended to be used from within file templates to provide an easy way to take boolean values stored in Pillars or @@ -378,7 +379,7 @@ def boolstr(value, true='true', false='false'): use_tls: yes - ''' + """ if value: return true From d8931841b8587689ff33c942db7628ba3cba1ce0 Mon Sep 17 00:00:00 2001 From: Adam Mendlik Date: Wed, 12 Jun 2019 20:52:19 -0600 Subject: [PATCH 29/33] Add missing period from slsutil.banner text --- salt/modules/slsutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/slsutil.py b/salt/modules/slsutil.py index 1433e4eedc1c..7d146b974a0d 100644 --- a/salt/modules/slsutil.py +++ b/salt/modules/slsutil.py @@ -311,7 +311,7 @@ def banner( text = ( "The contents of this file are managed by Salt. " "Any changes to this file may be overwritten " - "automatically and without warning" + "automatically and without warning." ) # Set up some typesetting variables From 1b934bb1031adccdaa57168c57545992b273dedf Mon Sep 17 00:00:00 2001 From: Adam Mendlik Date: Fri, 14 Jun 2019 16:42:18 -0600 Subject: [PATCH 30/33] Add support for Javadoc-style banners in slsutil.banner --- salt/modules/slsutil.py | 32 +++++++++++++++++++++++++----- tests/unit/modules/test_slsutil.py | 3 ++- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/salt/modules/slsutil.py b/salt/modules/slsutil.py index 7d146b974a0d..68e7e7b78c07 100644 --- a/salt/modules/slsutil.py +++ b/salt/modules/slsutil.py @@ -302,6 +302,24 @@ def banner( # The contents of this file are managed by Salt. Any changes to this # # file may be overwritten automatically and without warning. # ######################################################################## + + For a Javadoc-style banner: + + .. code-block:: jinja + {{ salt['slsutil.banner'](commentchar=' *',borderchar='*',blockstart='/**',blockend=' */') }} + + .. code-block:: none + + /** + *********************************************************************** + * * + * THIS FILE IS MANAGED BY SALT - DO NOT EDIT * + * * + * The contents of this file are managed by Salt. Any changes to this * + * file may be overwritten automatically and without warning. * + *********************************************************************** + */ + """ if title is None: @@ -315,17 +333,21 @@ def banner( ) # Set up some typesetting variables - lgutter = commentchar.strip() + " " - rgutter = " " + commentchar.strip() + ledge = commentchar.rstrip() + redge = commentchar.strip() + lgutter = ledge + " " + rgutter = " " + redge textwidth = width - len(lgutter) - len(rgutter) + + # Define the static elements border_line = ( - commentchar + borderchar[:1] * (width - len(commentchar) * 2) + commentchar + commentchar + borderchar[:1] * (width - len(ledge) - len(redge)) + redge ) spacer_line = commentchar + " " * (width - len(commentchar) * 2) + commentchar - wrapper = textwrap.TextWrapper(width=(width - len(lgutter) - len(rgutter))) - block = list() # Create the banner + wrapper = textwrap.TextWrapper(width=(width - len(lgutter) - len(rgutter))) + block = list() if blockstart is not None: block.append(blockstart) block.append(border_line) diff --git a/tests/unit/modules/test_slsutil.py b/tests/unit/modules/test_slsutil.py index 48a84f75730c..0762d1997d64 100644 --- a/tests/unit/modules/test_slsutil.py +++ b/tests/unit/modules/test_slsutil.py @@ -24,6 +24,7 @@ def test_banner(self): self.check_banner(width=20) self.check_banner(commentchar="//", borderchar="-") self.check_banner(title="title here", text="text here") + self.check_banner(commentchar=" *") def check_banner( self, @@ -50,7 +51,7 @@ def check_banner( for line in result: self.assertEqual(len(line), width) self.assertTrue(line.startswith(commentchar)) - self.assertTrue(line.endswith(commentchar)) + self.assertTrue(line.endswith(commentchar.strip())) def test_boolstr(self): """ From 9f3b7b8d82abd0e2025503190bbb5e5f266af6c4 Mon Sep 17 00:00:00 2001 From: Adam Mendlik Date: Sat, 15 Jun 2019 08:24:12 -0600 Subject: [PATCH 31/33] Add example documentation for slsutil.banner --- salt/modules/slsutil.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/salt/modules/slsutil.py b/salt/modules/slsutil.py index 68e7e7b78c07..fb43ee9c4a83 100644 --- a/salt/modules/slsutil.py +++ b/salt/modules/slsutil.py @@ -285,13 +285,11 @@ def banner( :param newline: Boolean value to indicate whether the comment block should end with a newline. Default is ``False``. - This banner can be injected into any templated file, for example: + **Example 1 - the default banner:** .. code-block:: jinja - {{ salt['slsutil.banner'](width=120, commentchar='//') }} - - The default banner: + {{ salt['slsutil.banner']() }} .. code-block:: none @@ -303,10 +301,11 @@ def banner( # file may be overwritten automatically and without warning. # ######################################################################## - For a Javadoc-style banner: + **Example 2 - a Javadoc-style banner:** .. code-block:: jinja - {{ salt['slsutil.banner'](commentchar=' *',borderchar='*',blockstart='/**',blockend=' */') }} + + {{ salt['slsutil.banner'](commentchar=' *', borderchar='*', blockstart='/**', blockend=' */') }} .. code-block:: none @@ -320,6 +319,23 @@ def banner( *********************************************************************** */ + **Example 3 - custom text:** + + .. code-block:: jinja + + {{ set copyright='This file may not be copied or distributed without the express permission of ACME Corp.' }} + {{ salt['slsutil.banner'](title='Copyright 2019 ACME Corp', text=copyright) }} + + .. code-block:: none + + ######################################################################## + # # + # Copyright 2019 ACME Corp # + # # + # This file may not be copied or distributed without the express # + # permission of ACME Corp. # + ######################################################################## + """ if title is None: From 97dc72c11186ca0c36fcfa4bcad110a7ac62d9af Mon Sep 17 00:00:00 2001 From: Adam Mendlik Date: Sat, 15 Jun 2019 08:50:29 -0600 Subject: [PATCH 32/33] Handle exception in slsutil.banner where width is too small to render --- salt/modules/slsutil.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/salt/modules/slsutil.py b/salt/modules/slsutil.py index fb43ee9c4a83..0dfc47e67bb1 100644 --- a/salt/modules/slsutil.py +++ b/salt/modules/slsutil.py @@ -355,6 +355,10 @@ def banner( rgutter = " " + redge textwidth = width - len(lgutter) - len(rgutter) + # Check the width + if textwidth <= 0: + raise salt.exceptions.ArgumentValueError("Width is too small to render banner") + # Define the static elements border_line = ( commentchar + borderchar[:1] * (width - len(ledge) - len(redge)) + redge @@ -362,7 +366,7 @@ def banner( spacer_line = commentchar + " " * (width - len(commentchar) * 2) + commentchar # Create the banner - wrapper = textwrap.TextWrapper(width=(width - len(lgutter) - len(rgutter))) + wrapper = textwrap.TextWrapper(width=textwidth) block = list() if blockstart is not None: block.append(blockstart) From aa3c636da81c1f122e316f9526058c054d27bfd8 Mon Sep 17 00:00:00 2001 From: Adam Mendlik Date: Tue, 30 Jul 2019 18:22:14 -0600 Subject: [PATCH 33/33] Update slsutil.banner example documentation --- salt/modules/slsutil.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/salt/modules/slsutil.py b/salt/modules/slsutil.py index 0dfc47e67bb1..13a674ff1a11 100644 --- a/salt/modules/slsutil.py +++ b/salt/modules/slsutil.py @@ -323,18 +323,18 @@ def banner( .. code-block:: jinja - {{ set copyright='This file may not be copied or distributed without the express permission of ACME Corp.' }} - {{ salt['slsutil.banner'](title='Copyright 2019 ACME Corp', text=copyright) }} + {{ set copyright='This file may not be copied or distributed without permission of SaltStack, Inc.' }} + {{ salt['slsutil.banner'](title='Copyright 2019 SaltStack, Inc.', text=copyright, width=60) }} .. code-block:: none - ######################################################################## - # # - # Copyright 2019 ACME Corp # - # # - # This file may not be copied or distributed without the express # - # permission of ACME Corp. # - ######################################################################## + ############################################################ + # # + # Copyright 2019 SaltStack, Inc. # + # # + # This file may not be copied or distributed without # + # permission of SaltStack, Inc. # + ############################################################ """