From bd1fe03ce72213e7a36a61d2ba5d433c6d488339 Mon Sep 17 00:00:00 2001 From: rallytime Date: Tue, 14 Feb 2017 13:33:04 -0700 Subject: [PATCH 01/12] Update :depends: docs for boto states and modules Some boto states and modules declare that they depend on the boto3 lib. That's all good. But some of these states and modules _also_ depend on the regular boto lib. This updates the docs to reflect this. Fixes #39304 --- salt/modules/boto_cloudtrail.py | 8 ++++++-- salt/modules/boto_iot.py | 8 ++++++-- salt/modules/boto_lambda.py | 8 ++++++-- salt/modules/boto_s3_bucket.py | 8 ++++++-- salt/states/boto_cloudtrail.py | 6 +++++- salt/states/boto_iot.py | 6 +++++- salt/states/boto_lambda.py | 6 +++++- salt/states/boto_s3_bucket.py | 6 +++++- 8 files changed, 44 insertions(+), 12 deletions(-) diff --git a/salt/modules/boto_cloudtrail.py b/salt/modules/boto_cloudtrail.py index ea76a0fcbc..25a251f8ac 100644 --- a/salt/modules/boto_cloudtrail.py +++ b/salt/modules/boto_cloudtrail.py @@ -4,6 +4,12 @@ Connection module for Amazon CloudTrail .. versionadded:: 2016.3.0 +:depends: + - boto + - boto3 + +The dependencies listed above can be installed via package or pip. + :configuration: This module accepts explicit Lambda credentials but can also utilize IAM roles assigned to the instance through Instance Profiles. Dynamic credentials are then automatically obtained from AWS API and no @@ -39,8 +45,6 @@ Connection module for Amazon CloudTrail key: askdjghsdfjkghWupUjasdflkdfklgjsdfjajkghs region: us-east-1 -:depends: boto3 - ''' # keep lint from choking on _get_conn and _cache_id #pylint: disable=E0602 diff --git a/salt/modules/boto_iot.py b/salt/modules/boto_iot.py index a72eb9ca3b..faa3a87c63 100644 --- a/salt/modules/boto_iot.py +++ b/salt/modules/boto_iot.py @@ -4,6 +4,12 @@ Connection module for Amazon IoT .. versionadded:: 2016.3.0 +:depends: + - boto + - boto3 + +The dependencies listed above can be installed via package or pip. + :configuration: This module accepts explicit Lambda credentials but can also utilize IAM roles assigned to the instance through Instance Profiles. Dynamic credentials are then automatically obtained from AWS API and no @@ -39,8 +45,6 @@ Connection module for Amazon IoT key: askdjghsdfjkghWupUjasdflkdfklgjsdfjajkghs region: us-east-1 -:depends: boto3 - ''' # keep lint from choking on _get_conn and _cache_id #pylint: disable=E0602 diff --git a/salt/modules/boto_lambda.py b/salt/modules/boto_lambda.py index 47832ede28..549b972854 100644 --- a/salt/modules/boto_lambda.py +++ b/salt/modules/boto_lambda.py @@ -4,6 +4,12 @@ Connection module for Amazon Lambda .. versionadded:: 2016.3.0 +:depends: + - boto + - boto3 + +The dependencies listed above can be installed via package or pip. + :configuration: This module accepts explicit Lambda credentials but can also utilize IAM roles assigned to the instance through Instance Profiles. Dynamic credentials are then automatically obtained from AWS API and no @@ -69,8 +75,6 @@ Connection module for Amazon Lambda error: message: error message -:depends: boto3 - ''' # keep lint from choking on _get_conn and _cache_id #pylint: disable=E0602 diff --git a/salt/modules/boto_s3_bucket.py b/salt/modules/boto_s3_bucket.py index 3226256fab..1777d02e8c 100644 --- a/salt/modules/boto_s3_bucket.py +++ b/salt/modules/boto_s3_bucket.py @@ -4,6 +4,12 @@ Connection module for Amazon S3 Buckets .. versionadded:: 2016.3.0 +:depends: + - boto + - boto3 + +The dependencies listed above can be installed via package or pip. + :configuration: This module accepts explicit Lambda credentials but can also utilize IAM roles assigned to the instance through Instance Profiles. Dynamic credentials are then automatically obtained from AWS API and no @@ -39,8 +45,6 @@ Connection module for Amazon S3 Buckets key: askdjghsdfjkghWupUjasdflkdfklgjsdfjajkghs region: us-east-1 -:depends: boto3 - ''' # keep lint from choking on _get_conn and _cache_id #pylint: disable=E0602 diff --git a/salt/states/boto_cloudtrail.py b/salt/states/boto_cloudtrail.py index 52b79654f4..1da56b4838 100644 --- a/salt/states/boto_cloudtrail.py +++ b/salt/states/boto_cloudtrail.py @@ -8,7 +8,11 @@ Manage CloudTrail Objects Create and destroy CloudTrail objects. Be aware that this interacts with Amazon's services, and so may incur charges. -This module uses ``boto3``, which can be installed via package, or pip. +:depends: + - boto + - boto3 + +The dependencies listed above can be installed via package or pip. This module accepts explicit vpc credentials but can also utilize IAM roles assigned to the instance through Instance Profiles. Dynamic diff --git a/salt/states/boto_iot.py b/salt/states/boto_iot.py index ca5f8dcf09..adbf629ebc 100644 --- a/salt/states/boto_iot.py +++ b/salt/states/boto_iot.py @@ -8,7 +8,11 @@ Manage IoT Objects Create and destroy IoT objects. Be aware that this interacts with Amazon's services, and so may incur charges. -This module uses ``boto3``, which can be installed via package, or pip. +:depends: + - boto + - boto3 + +The dependencies listed above can be installed via package or pip. This module accepts explicit vpc credentials but can also utilize IAM roles assigned to the instance through Instance Profiles. Dynamic diff --git a/salt/states/boto_lambda.py b/salt/states/boto_lambda.py index 26f369122f..ca79f50099 100644 --- a/salt/states/boto_lambda.py +++ b/salt/states/boto_lambda.py @@ -8,7 +8,11 @@ Manage Lambda Functions Create and destroy Lambda Functions. Be aware that this interacts with Amazon's services, and so may incur charges. -This module uses ``boto3``, which can be installed via package, or pip. +:depends: + - boto + - boto3 + +The dependencies listed above can be installed via package or pip. This module accepts explicit vpc credentials but can also utilize IAM roles assigned to the instance through Instance Profiles. Dynamic diff --git a/salt/states/boto_s3_bucket.py b/salt/states/boto_s3_bucket.py index 21e8d68d2f..b6e52b241d 100644 --- a/salt/states/boto_s3_bucket.py +++ b/salt/states/boto_s3_bucket.py @@ -8,7 +8,11 @@ Manage S3 Buckets Create and destroy S3 buckets. Be aware that this interacts with Amazon's services, and so may incur charges. -This module uses ``boto3``, which can be installed via package, or pip. +:depends: + - boto + - boto3 + +The dependencies listed above can be installed via package or pip. This module accepts explicit vpc credentials but can also utilize IAM roles assigned to the instance through Instance Profiles. Dynamic From e13febe58d1f14473d2893f20d43a1707bf94851 Mon Sep 17 00:00:00 2001 From: rallytime Date: Tue, 14 Feb 2017 17:18:35 -0700 Subject: [PATCH 02/12] Update external_cache docs with other configuration options There are more places than just the minion config file to define external job cache configuration settings for minions. This change updates the docs to include some of those other places and the order in which they're evaluated if the settings are defined in more than one place. Fixes #38762 --- doc/topics/jobs/external_cache.rst | 31 +++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/doc/topics/jobs/external_cache.rst b/doc/topics/jobs/external_cache.rst index ff5c6fe750..ef83e94df0 100644 --- a/doc/topics/jobs/external_cache.rst +++ b/doc/topics/jobs/external_cache.rst @@ -89,12 +89,33 @@ A simpler returner, such as Slack or HipChat, requires: Step 2: Configure the Returner ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -After you understand the configuration and have the external system ready, add -the returner configuration settings to the Salt Minion configuration file for -the External Job Cache, or to the Salt Master configuration file for the Master -Job Cache. +After you understand the configuration and have the external system ready, the +configuration requirements must be declared. -For example, MySQL requires: +External Job Cache +"""""""""""""""""" + +The returner configuration settings can be declared in the Salt Minion +configuration file, the Minion's pillar data, or the Minion's grains. + +If ``external_job_cache`` configuration settings are specified in more than +one place, the options are retrieved in the following order. The first +configuration location that is found is the one that will be used. + +- Minion configuration file +- Minion's grains +- Minion's pillar data + +Master Job Cache +"""""""""""""""" + +The returner configuration settings for the Master Job Cache should be +declared in the Salt Master's configuration file. + +Configuration File Examples +""""""""""""""""""""""""""" + +MySQL requires: .. code-block:: yaml From 7e1803b617cd9d594a0df3f07ae55fac14b91693 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Wed, 15 Feb 2017 11:10:39 -0600 Subject: [PATCH 03/12] Update docs on upstream EPEL7 pygit2/libgit2 issues (#39421) pygit2 was rebuilt, but libgit2 still needs a rebuild. I opened an upstream bug report this morning, this updates the docs to reflect the current status of this upstream issue. --- doc/topics/tutorials/gitfs.rst | 55 ++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/doc/topics/tutorials/gitfs.rst b/doc/topics/tutorials/gitfs.rst index 6d74ab3040..2e411956bb 100644 --- a/doc/topics/tutorials/gitfs.rst +++ b/doc/topics/tutorials/gitfs.rst @@ -100,11 +100,13 @@ releases pygit2_ 0.20.3 and libgit2_ 0.20.0 is the recommended combination. RedHat Pygit2 Issues ~~~~~~~~~~~~~~~~~~~~ -Around the time of the release of RedHat 7.3, RedHat effectively broke pygit2_ -by upgrading python-cffi_ to a release incompatible with the version of pygit2_ -available in their repositories. This prevents Python from importing the -pygit2_ module at all, leading to a master that refuses to start, and leaving -the following errors in the master log file: +The release of RedHat/CentOS 7.3 upgraded both ``python-cffi`` and +``http-parser``, both of which are dependencies for pygit2_/libgit2_. Both +pygit2_ and libgit2_ (which are from the EPEL repository and not managed +directly by RedHat) need to be rebuilt against these updated dependencies. + +The below errors will show up in the master log if an incompatible +``python-pygit2`` package is installed: .. code-block:: text @@ -113,34 +115,37 @@ the following errors in the master log file: 2017-02-10 09:07:34,907 [salt.utils.gitfs ][CRITICAL][11211] No suitable gitfs provider module is installed. 2017-02-10 09:07:34,912 [salt.master ][CRITICAL][11211] Master failed pre flight checks, exiting -This issue has been reported on the `RedHat Bugzilla`_. In the meantime, you -can work around it by downgrading python-cffi_. To do this, go to `this page`_ -and download the appropriate python-cffi_ 0.8.6 RPM. Then copy that RPM to the -master and downgrade using the ``rpm`` command. For example: +The below errors will show up in the master log if an incompatible ``libgit2`` +package is installed: + +.. code-block:: text + + 2017-02-15 18:04:45,211 [salt.utils.gitfs ][ERROR ][6211] Error occurred fetching gitfs remote 'https://foo.com/bar.git': No Content-Type header in response + +As of 15 February 2017, ``python-pygit2`` has been rebuilt and is in the stable +EPEL repository. However, ``libgit2`` remains broken (a `bug report`_ has been +filed to get it rebuilt). + +In the meantime, you can work around this by downgrading ``http-parser``. To do +this, go to `this page`_ and download the appropriate ``http-parser`` RPM for +the OS architecture you are using (x86_64, etc.). Then downgrade using the +``rpm`` command. For example: .. code-block:: bash - # rpm -Uvh --oldpackage python-cffi-0.8.6-1.el7.x86_64.rpm + [root@784e8a8c5028 /]# curl --silent -O https://kojipkgs.fedoraproject.org//packages/http-parser/2.0/5.20121128gitcd01361.el7/x86_64/http-parser-2.0-5.20121128gitcd01361.el7.x86_64.rpm + [root@784e8a8c5028 /]# rpm -Uvh --oldpackage http-parser-2.0-5.20121128gitcd01361.el7.x86_64.rpm Preparing... ################################# [100%] Updating / installing... - 1:python-cffi-0.8.6-1.el7 ################################# [ 50%] + 1:http-parser-2.0-5.20121128gitcd01################################# [ 50%] Cleaning up / removing... - 2:python-cffi-1.6.0-5.el7 ################################# [100%] - # rpm -q python-cffi - python-cffi-0.8.6-1.el7.x86_64 + 2:http-parser-2.7.1-3.el7 ################################# [100%] -To confirm that pygit2_ is now "fixed", you can test trying to import it like so: +A restart of the salt-master daemon may be required to allow http(s) +repositories to continue to be fetched. -.. code-block:: bash - - # python -c 'import pygit2' - # - -If the command produces no output, then your master should work when you start -it again. - -.. _`this page`: https://koji.fedoraproject.org/koji/buildinfo?buildID=569520 -.. _`RedHat Bugzilla`: https://bugzilla.redhat.com/show_bug.cgi?id=1400668 +.. _`this page`: https://koji.fedoraproject.org/koji/buildinfo?buildID=703753 +.. _`bug report`: https://bugzilla.redhat.com/show_bug.cgi?id=1422583 GitPython From a7d5118262716daae886d80d6a364557aab10cc4 Mon Sep 17 00:00:00 2001 From: Morgan Willcock Date: Wed, 15 Feb 2017 17:58:59 +0000 Subject: [PATCH 04/12] Return failure when package path does not exist --- salt/states/win_dism.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/salt/states/win_dism.py b/salt/states/win_dism.py index fa3ea59134..4b73d4e711 100644 --- a/salt/states/win_dism.py +++ b/salt/states/win_dism.py @@ -17,6 +17,7 @@ from __future__ import absolute_import # Import python libs import logging +import os # Import salt libs import salt.utils @@ -319,6 +320,15 @@ def package_installed(name, 'comment': '', 'changes': {}} + # Fail if using a non-existent package path + if '~' not in name and not os.path.exists(name): + if __opts__['test']: + ret['result'] = None + else: + ret['result'] = False + ret['comment'] = 'Package path {0} does not exist'.format(name) + return ret + old = __salt__['dism.installed_packages']() # Get package info so we can see if it's already installed @@ -387,6 +397,15 @@ def package_removed(name, image=None, restart=False): 'comment': '', 'changes': {}} + # Fail if using a non-existent package path + if '~' not in name and not os.path.exists(name): + if __opts__['test']: + ret['result'] = None + else: + ret['result'] = False + ret['comment'] = 'Package path {0} does not exist'.format(name) + return ret + old = __salt__['dism.installed_packages']() # Get package info so we can see if it's already removed From 709c197f84dc722ad6d17c15a87b33a61de7acb8 Mon Sep 17 00:00:00 2001 From: David Boucha Date: Wed, 15 Feb 2017 13:42:18 -0700 Subject: [PATCH 05/12] allow sync_grains to be disabled on grains.setval --- salt/modules/grains.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/salt/modules/grains.py b/salt/modules/grains.py index e87f818ed6..bd9069e396 100644 --- a/salt/modules/grains.py +++ b/salt/modules/grains.py @@ -192,7 +192,7 @@ def item(*args, **kwargs): return ret -def setvals(grains, destructive=False): +def setvals(grains, destructive=False, refresh=True): ''' Set new grains values in the grains config file @@ -275,12 +275,12 @@ def setvals(grains, destructive=False): log.error(msg.format(fn_)) if not __opts__.get('local', False): # Sync the grains - __salt__['saltutil.sync_grains']() + __salt__['saltutil.sync_grains'](refresh=refresh) # Return the grains we just set to confirm everything was OK return new_grains -def setval(key, val, destructive=False): +def setval(key, val, destructive=False, refresh=True): ''' Set a grains value in the grains config file @@ -301,7 +301,7 @@ def setval(key, val, destructive=False): salt '*' grains.setval key val salt '*' grains.setval key "{'sub-key': 'val', 'sub-key2': 'val2'}" ''' - return setvals({key: val}, destructive) + return setvals({key: val}, destructive, refresh) def append(key, val, convert=False, delimiter=DEFAULT_TARGET_DELIM): From 391bbecd9056bad267d1490dc772c8b011e3e159 Mon Sep 17 00:00:00 2001 From: David Boucha Date: Wed, 15 Feb 2017 13:49:24 -0700 Subject: [PATCH 06/12] add docs --- salt/modules/grains.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/salt/modules/grains.py b/salt/modules/grains.py index bd9069e396..2082dc03d0 100644 --- a/salt/modules/grains.py +++ b/salt/modules/grains.py @@ -200,6 +200,10 @@ def setvals(grains, destructive=False, refresh=True): If an operation results in a key being removed, delete the key, too. Defaults to False. + refresh + Refresh minion grains using saltutil.sync_grains. + Defaults to True. + CLI Example: .. code-block:: bash @@ -294,6 +298,10 @@ def setval(key, val, destructive=False, refresh=True): If an operation results in a key being removed, delete the key, too. Defaults to False. + refresh + Refresh minion grains using saltutil.sync_grains. + Defaults to True. + CLI Example: .. code-block:: bash From e652a455923caf9a1b786e3151bebf1d1437a220 Mon Sep 17 00:00:00 2001 From: Mike Place Date: Thu, 16 Feb 2017 12:26:49 -0700 Subject: [PATCH 07/12] Fix mocks in win_disim tests --- tests/unit/states/win_dism_test.py | 73 +++++++++++++++++------------- 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/tests/unit/states/win_dism_test.py b/tests/unit/states/win_dism_test.py index 9d2811ff10..367627630b 100644 --- a/tests/unit/states/win_dism_test.py +++ b/tests/unit/states/win_dism_test.py @@ -95,11 +95,12 @@ class WinDismTestCase(TestCase): dism.__salt__, {'dism.installed_capabilities': mock_installed, 'dism.add_capability': mock_add}): - out = dism.capability_installed('Capa2', 'somewhere', True) + with patch.dict(dism.__opts__, {'test': False}): + out = dism.capability_installed('Capa2', 'somewhere', True) - mock_installed.assert_called_once_with() - assert not mock_add.called - self.assertEqual(out, expected) + mock_installed.assert_called_once_with() + assert not mock_add.called + self.assertEqual(out, expected) def test_capability_removed(self): ''' @@ -360,13 +361,14 @@ class WinDismTestCase(TestCase): 'dism.add_package': mock_add, 'dism.package_info': mock_info}): with patch.dict(dism.__opts__, {'test': False}): + with patch('os.path.exists'): - out = dism.package_installed('Pack2') + out = dism.package_installed('Pack2') - mock_installed.assert_called_with() - mock_add.assert_called_once_with( - 'Pack2', False, False, None, False) - self.assertEqual(out, expected) + mock_installed.assert_called_with() + mock_add.assert_called_once_with( + 'Pack2', False, False, None, False) + self.assertEqual(out, expected) def test_package_installed_failure(self): ''' @@ -390,13 +392,14 @@ class WinDismTestCase(TestCase): 'dism.add_package': mock_add, 'dism.package_info': mock_info}): with patch.dict(dism.__opts__, {'test': False}): + with patch('os.path.exists'): - out = dism.package_installed('Pack2') + out = dism.package_installed('Pack2') - mock_installed.assert_called_with() - mock_add.assert_called_once_with( - 'Pack2', False, False, None, False) - self.assertEqual(out, expected) + mock_installed.assert_called_with() + mock_add.assert_called_once_with( + 'Pack2', False, False, None, False) + self.assertEqual(out, expected) def test_package_installed_installed(self): ''' @@ -418,12 +421,14 @@ class WinDismTestCase(TestCase): dism.__salt__, {'dism.installed_packages': mock_installed, 'dism.add_package': mock_add, 'dism.package_info': mock_info}): + with patch.dict(dism.__opts__, {'test': False}): + with patch('os.path.exists'): - out = dism.package_installed('Pack2') + out = dism.package_installed('Pack2') - mock_installed.assert_called_once_with() - assert not mock_add.called - self.assertEqual(out, expected) + mock_installed.assert_called_once_with() + assert not mock_add.called + self.assertEqual(out, expected) def test_package_removed(self): ''' @@ -448,13 +453,14 @@ class WinDismTestCase(TestCase): 'dism.remove_package': mock_remove, 'dism.package_info': mock_info}): with patch.dict(dism.__opts__, {'test': False}): + with patch('os.path.exists'): - out = dism.package_removed('Pack2') + out = dism.package_removed('Pack2') - mock_removed.assert_called_with() - mock_remove.assert_called_once_with( - 'Pack2', None, False) - self.assertEqual(out, expected) + mock_removed.assert_called_with() + mock_remove.assert_called_once_with( + 'Pack2', None, False) + self.assertEqual(out, expected) def test_package_removed_failure(self): ''' @@ -478,13 +484,14 @@ class WinDismTestCase(TestCase): 'dism.remove_package': mock_remove, 'dism.package_info': mock_info}): with patch.dict(dism.__opts__, {'test': False}): + with patch('os.path.exists'): - out = dism.package_removed('Pack2') + out = dism.package_removed('Pack2') - mock_removed.assert_called_with() - mock_remove.assert_called_once_with( - 'Pack2', None, False) - self.assertEqual(out, expected) + mock_removed.assert_called_with() + mock_remove.assert_called_once_with( + 'Pack2', None, False) + self.assertEqual(out, expected) def test_package_removed_removed(self): ''' @@ -507,11 +514,13 @@ class WinDismTestCase(TestCase): 'dism.remove_package': mock_remove, 'dism.package_info': mock_info}): - out = dism.package_removed('Pack2') + with patch.dict(dism.__opts__, {'test': False}): + with patch('os.path.exists'): + out = dism.package_removed('Pack2') - mock_removed.assert_called_once_with() - assert not mock_remove.called - self.assertEqual(out, expected) + mock_removed.assert_called_once_with() + assert not mock_remove.called + self.assertEqual(out, expected) if __name__ == '__main__': From f829d6f9fc4ffa97b2c175a4e915a174631eba72 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Thu, 16 Feb 2017 15:10:43 -0600 Subject: [PATCH 08/12] skip false values from preferred_ip --- salt/cloud/clouds/dimensiondata.py | 2 ++ salt/cloud/clouds/nova.py | 2 ++ salt/cloud/clouds/openstack.py | 2 ++ salt/cloud/clouds/rackspace.py | 2 ++ 4 files changed, 8 insertions(+) diff --git a/salt/cloud/clouds/dimensiondata.py b/salt/cloud/clouds/dimensiondata.py index 9b43815e8c..48924c136e 100644 --- a/salt/cloud/clouds/dimensiondata.py +++ b/salt/cloud/clouds/dimensiondata.py @@ -228,6 +228,8 @@ def create(vm_): ) for private_ip in private: private_ip = preferred_ip(vm_, [private_ip]) + if private_ip is False: + continue if salt.utils.cloud.is_public_ip(private_ip): log.warning('%s is a public IP', private_ip) data.public_ips.append(private_ip) diff --git a/salt/cloud/clouds/nova.py b/salt/cloud/clouds/nova.py index 2af1713a26..fda1ba6b81 100644 --- a/salt/cloud/clouds/nova.py +++ b/salt/cloud/clouds/nova.py @@ -833,6 +833,8 @@ def create(vm_): ) for private_ip in private: private_ip = preferred_ip(vm_, [private_ip]) + if private_ip is False: + continue if salt.utils.cloud.is_public_ip(private_ip): log.warn('{0} is a public IP'.format(private_ip)) data.public_ips.append(private_ip) diff --git a/salt/cloud/clouds/openstack.py b/salt/cloud/clouds/openstack.py index f3da373360..6e3b9b7a5c 100644 --- a/salt/cloud/clouds/openstack.py +++ b/salt/cloud/clouds/openstack.py @@ -718,6 +718,8 @@ def create(vm_): ) for private_ip in private: private_ip = preferred_ip(vm_, [private_ip]) + if private_ip is False: + continue if salt.utils.cloud.is_public_ip(private_ip): log.warn('{0} is a public IP'.format(private_ip)) data.public_ips.append(private_ip) diff --git a/salt/cloud/clouds/rackspace.py b/salt/cloud/clouds/rackspace.py index abdeaeee2e..300151faee 100644 --- a/salt/cloud/clouds/rackspace.py +++ b/salt/cloud/clouds/rackspace.py @@ -286,6 +286,8 @@ def create(vm_): ) for private_ip in private: private_ip = preferred_ip(vm_, [private_ip]) + if private_ip is False: + continue if salt.utils.cloud.is_public_ip(private_ip): log.warn('{0} is a public IP'.format(private_ip)) data.public_ips.append(private_ip) From 9d13422ac14f7e81769a60ec7179b6edc2064d7c Mon Sep 17 00:00:00 2001 From: Mihai Dinca Date: Fri, 10 Feb 2017 09:22:17 +0100 Subject: [PATCH 09/12] OpenSCAP module --- salt/modules/openscap.py | 105 ++++++++++++++ tests/unit/modules/openscap_test.py | 207 ++++++++++++++++++++++++++++ 2 files changed, 312 insertions(+) create mode 100644 salt/modules/openscap.py create mode 100644 tests/unit/modules/openscap_test.py diff --git a/salt/modules/openscap.py b/salt/modules/openscap.py new file mode 100644 index 0000000000..b50ede7c28 --- /dev/null +++ b/salt/modules/openscap.py @@ -0,0 +1,105 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import +import tempfile +import shlex +import shutil +from subprocess import Popen, PIPE + +from salt.client import Caller + + +ArgumentParser = object + +try: + import argparse # pylint: disable=minimum-python-version + ArgumentParser = argparse.ArgumentParser + HAS_ARGPARSE = True +except ImportError: # python 2.6 + HAS_ARGPARSE = False + + +_XCCDF_MAP = { + 'eval': { + 'parser_arguments': [ + (('--profile',), {'required': True}), + ], + 'cmd_pattern': ( + "oscap xccdf eval " + "--oval-results --results results.xml --report report.html " + "--profile {0} {1}" + ) + } +} + + +def __virtual__(): + return HAS_ARGPARSE, 'argparse module is required.' + + +class _ArgumentParser(ArgumentParser): + + def __init__(self, action=None, *args, **kwargs): + super(_ArgumentParser, self).__init__(*args, prog='oscap', **kwargs) + self.add_argument('action', choices=['eval']) + add_arg = None + for params, kwparams in _XCCDF_MAP['eval']['parser_arguments']: + self.add_argument(*params, **kwparams) + + def error(self, message, *args, **kwargs): + raise Exception(message) + + +_OSCAP_EXIT_CODES_MAP = { + 0: True, # all rules pass + 1: False, # there is an error during evaluation + 2: True # there is at least one rule with either fail or unknown result +} + + +def xccdf(params): + ''' + Run ``oscap xccdf`` commands on minions. + It uses cp.push_dir to upload the generated files to the salt master + in the master's minion files cachedir + (defaults to ``/var/cache/salt/master/minions/minion-id/files``) + + It needs ``file_recv`` set to ``True`` in the master configuration file. + + CLI Example: + + .. code-block:: bash + + salt '*' openscap.xccdf "eval --profile Default /usr/share/openscap/scap-yast2sec-xccdf.xml" + ''' + params = shlex.split(params) + policy = params[-1] + + success = True + error = None + upload_dir = None + action = None + + try: + parser = _ArgumentParser() + action = parser.parse_known_args(params)[0].action + args, argv = _ArgumentParser(action=action).parse_known_args(args=params) + except Exception as err: + success = False + error = str(err) + + if success: + cmd = _XCCDF_MAP[action]['cmd_pattern'].format(args.profile, policy) + tempdir = tempfile.mkdtemp() + proc = Popen( + shlex.split(cmd), stdout=PIPE, stderr=PIPE, cwd=tempdir) + (stdoutdata, stderrdata) = proc.communicate() + success = _OSCAP_EXIT_CODES_MAP[proc.returncode] + if success: + caller = Caller() + caller.cmd('cp.push_dir', tempdir) + shutil.rmtree(tempdir, ignore_errors=True) + upload_dir = tempdir + else: + error = stderrdata + + return dict(success=success, upload_dir=upload_dir, error=error) diff --git a/tests/unit/modules/openscap_test.py b/tests/unit/modules/openscap_test.py new file mode 100644 index 0000000000..c883975690 --- /dev/null +++ b/tests/unit/modules/openscap_test.py @@ -0,0 +1,207 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import + +from subprocess import PIPE + +from salt.modules import openscap + +from salttesting import skipIf, TestCase +from salttesting.mock import ( + Mock, + MagicMock, + patch, + NO_MOCK, + NO_MOCK_REASON +) + + +@skipIf(NO_MOCK, NO_MOCK_REASON) +class OpenscapTestCase(TestCase): + + random_temp_dir = '/tmp/unique-name' + policy_file = '/usr/share/openscap/policy-file-xccdf.xml' + + def setUp(self): + patchers = [ + patch('salt.modules.openscap.Caller', MagicMock()), + patch('salt.modules.openscap.shutil.rmtree', Mock()), + patch( + 'salt.modules.openscap.tempfile.mkdtemp', + Mock(return_value=self.random_temp_dir) + ), + ] + for patcher in patchers: + self.apply_patch(patcher) + + def apply_patch(self, patcher): + patcher.start() + self.addCleanup(patcher.stop) + + @patch( + 'salt.modules.openscap.Popen', + MagicMock( + return_value=Mock( + **{'returncode': 0, 'communicate.return_value': ('', '')} + ) + ) + ) + def test_openscap_xccdf_eval_success(self): + response = openscap.xccdf( + 'eval --profile Default {0}'.format(self.policy_file)) + + self.assertEqual(openscap.tempfile.mkdtemp.call_count, 1) + expected_cmd = [ + 'oscap', + 'xccdf', + 'eval', + '--oval-results', + '--results', 'results.xml', + '--report', 'report.html', + '--profile', 'Default', + self.policy_file + ] + openscap.Popen.assert_called_once_with( + expected_cmd, + cwd=openscap.tempfile.mkdtemp.return_value, + stderr=PIPE, + stdout=PIPE) + openscap.Caller().cmd.assert_called_once_with( + 'cp.push_dir', self.random_temp_dir) + self.assertEqual(openscap.shutil.rmtree.call_count, 1) + self.assertEqual( + response, + { + 'upload_dir': self.random_temp_dir, + 'error': None, 'success': True + } + ) + + @patch( + 'salt.modules.openscap.Popen', + MagicMock( + return_value=Mock( + **{'returncode': 2, 'communicate.return_value': ('', '')} + ) + ) + ) + def test_openscap_xccdf_eval_success_with_failing_rules(self): + response = openscap.xccdf( + 'eval --profile Default {0}'.format(self.policy_file)) + + self.assertEqual(openscap.tempfile.mkdtemp.call_count, 1) + expected_cmd = [ + 'oscap', + 'xccdf', + 'eval', + '--oval-results', + '--results', 'results.xml', + '--report', 'report.html', + '--profile', 'Default', + self.policy_file + ] + openscap.Popen.assert_called_once_with( + expected_cmd, + cwd=openscap.tempfile.mkdtemp.return_value, + stderr=PIPE, + stdout=PIPE) + openscap.Caller().cmd.assert_called_once_with( + 'cp.push_dir', self.random_temp_dir) + self.assertEqual(openscap.shutil.rmtree.call_count, 1) + self.assertEqual( + response, + { + 'upload_dir': self.random_temp_dir, + 'error': None, + 'success': True + } + ) + + def test_openscap_xccdf_eval_fail_no_profile(self): + response = openscap.xccdf( + 'eval --param Default /unknown/param') + self.assertEqual( + response, + { + 'error': 'argument --profile is required', + 'upload_dir': None, + 'success': False + } + ) + + @patch( + 'salt.modules.openscap.Popen', + MagicMock( + return_value=Mock( + **{'returncode': 2, 'communicate.return_value': ('', '')} + ) + ) + ) + def test_openscap_xccdf_eval_success_ignore_unknown_params(self): + response = openscap.xccdf( + 'eval --profile Default --param Default /policy/file') + self.assertEqual( + response, + { + 'upload_dir': self.random_temp_dir, + 'error': None, + 'success': True + } + ) + expected_cmd = [ + 'oscap', + 'xccdf', + 'eval', + '--oval-results', + '--results', 'results.xml', + '--report', 'report.html', + '--profile', 'Default', + '/policy/file' + ] + openscap.Popen.assert_called_once_with( + expected_cmd, + cwd=openscap.tempfile.mkdtemp.return_value, + stderr=PIPE, + stdout=PIPE) + + @patch( + 'salt.modules.openscap.Popen', + MagicMock( + return_value=Mock(**{ + 'returncode': 1, + 'communicate.return_value': ('', 'evaluation error') + }) + ) + ) + def test_openscap_xccdf_eval_evaluation_error(self): + response = openscap.xccdf( + 'eval --profile Default {0}'.format(self.policy_file)) + + self.assertEqual( + response, + { + 'upload_dir': None, + 'error': 'evaluation error', + 'success': False + } + ) + + @patch( + 'salt.modules.openscap.Popen', + MagicMock( + return_value=Mock(**{ + 'returncode': 1, + 'communicate.return_value': ('', 'evaluation error') + }) + ) + ) + def test_openscap_xccdf_eval_fail_not_implemented_action(self): + response = openscap.xccdf('info {0}'.format(self.policy_file)) + + self.assertEqual( + response, + { + 'upload_dir': None, + 'error': "argument action: invalid choice: 'info' (choose from 'eval')", + 'success': False + } + ) From 20b097a74538584da995c8e94e47aa48907debf5 Mon Sep 17 00:00:00 2001 From: Tomas Zvala Date: Fri, 17 Feb 2017 17:22:14 +0100 Subject: [PATCH 10/12] dockerng: compare sets instead of lists of security_opt Apparently some versions of docker add label=disabled to security_opt when the container is launched as privileged. This causes Salt to relaunch the container to remove it on next run. Container started as privileged and with the security_opt set, causes it to have the option set twice and makes salt want to remove one instance. With this fix, dockerng will compare just (non-)existence of the flag. So containers started with privileged flag and security_opt set to label=disabled will not get relaunched on every salt run. Fixes #39447 --- salt/states/dockerng.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/salt/states/dockerng.py b/salt/states/dockerng.py index 9e4f04f315..358a5091e5 100644 --- a/salt/states/dockerng.py +++ b/salt/states/dockerng.py @@ -426,6 +426,21 @@ def _compare(actual, create_kwargs, defaults_from_image): if actual_data != data: ret.update({item: {'old': actual_data, 'new': data}}) continue + elif item == 'security_opt': + if actual_data is None: + actual_data = [] + if data is None: + data = [] + actual_data = sorted(set(actual_data)) + desired_data = sorted(set(data)) + log.trace('dockerng.running ({0}): munged actual value: {1}' + .format(item, actual_data)) + log.trace('dockerng.running ({0}): munged desired value: {1}' + .format(item, desired_data)) + if actual_data != desired_data: + ret.update({item: {'old': actual_data, + 'new': desired_data}}) + continue elif item in ('cmd', 'command', 'entrypoint'): if (actual_data is None and item not in create_kwargs and _image_get(config['image_path'])): From a6a17d58aaa5b478d25870037561d4a7e4fc233e Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Sat, 18 Feb 2017 19:37:46 -0600 Subject: [PATCH 11/12] Handle docker-py 2.0's new host_config path docker-py 2.0 made some changes to the location of the function which creates the host config. This adds a function to get the proper argspec for host config, networking config, and container config, irrespective of the installed version of docker-py. --- salt/modules/dockerng.py | 93 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 3 deletions(-) diff --git a/salt/modules/dockerng.py b/salt/modules/dockerng.py index f91b4e3af2..fe077228db 100644 --- a/salt/modules/dockerng.py +++ b/salt/modules/dockerng.py @@ -264,7 +264,6 @@ import distutils.version # pylint: disable=import-error,no-name-in-module,unuse import fnmatch import functools import gzip -import inspect as inspect_module import json import logging import os @@ -278,6 +277,7 @@ import time # Import Salt libs from salt.exceptions import CommandExecutionError, SaltInvocationError from salt.ext.six.moves import map # pylint: disable=import-error,redefined-builtin +from salt.utils.args import get_function_argspec as _argspec from salt.utils.decorators \ import identical_signature_wrapper as _mimic_signature import salt.utils @@ -288,11 +288,22 @@ import salt.ext.six as six # pylint: disable=import-error try: import docker - import docker.utils HAS_DOCKER_PY = True except ImportError: HAS_DOCKER_PY = False +# These next two imports are only necessary to have access to the needed +# functions so that we can get argspecs for the container config, host config, +# and networking config (see the get_client_args() function). +try: + import docker.types +except ImportError: + pass +try: + import docker.utils +except ImportError: + pass + try: PY_VERSION = sys.version_info[0] if PY_VERSION == 2: @@ -2994,7 +3005,7 @@ def create(image, # https://docs.docker.com/engine/reference/api/docker_remote_api_v1.15/#create-a-container if salt.utils.version_cmp(version()['ApiVersion'], '1.15') > 0: client = __context__['docker.client'] - host_config_args = inspect_module.getargspec(docker.utils.create_host_config).args + host_config_args = get_client_args()['host_config'] create_kwargs['host_config'] = client.create_host_config( **dict((arg, create_kwargs.pop(arg, None)) for arg in host_config_args if arg != 'version') ) @@ -5492,3 +5503,79 @@ def script_retcode(name, ignore_retcode=ignore_retcode, use_vt=use_vt, keep_env=keep_env)['retcode'] + + +def get_client_args(): + ''' + .. versionadded:: 2016.3.6,2016.11.4,Nitrogen + + Returns the args for docker-py's `low-level API`_, organized by container + config, host config, and networking config. This is designed for use by the + :mod:`docker states ` to more gracefully handle API + changes. + + .. _`low-level API`: http://docker-py.readthedocs.io/en/stable/api.html + + CLI Example: + + .. code-block:: bash + + salt myminion docker.get_client_args + ''' + try: + config_args = _argspec(docker.types.ContainerConfig.__init__).args + except AttributeError: + try: + config_args = _argspec(docker.utils.create_container_config).args + except AttributeError: + raise CommandExecutionError( + 'Failed to get create_container_config argspec' + ) + + try: + host_config_args = \ + _argspec(docker.types.HostConfig.__init__).args + except AttributeError: + try: + host_config_args = _argspec(docker.utils.create_host_config).args + except AttributeError: + raise CommandExecutionError( + 'Failed to get create_host_config argspec' + ) + + try: + endpoint_config_args = \ + _argspec(docker.types.EndpointConfig.__init__).args + except AttributeError: + try: + endpoint_config_args = \ + _argspec(docker.utils.create_endpoint_config).args + except AttributeError: + raise CommandExecutionError( + 'Failed to get create_host_config argspec' + ) + + for arglist in (config_args, host_config_args, endpoint_config_args): + try: + # The API version is passed automagically by the API code that + # imports these classes/functions and is not an arg that we will be + # passing, so remove it if present. + arglist.remove('version') + except ValueError: + pass + + # Remove any args in host or networking config from the main config dict. + # This keeps us from accidentally allowing args that have been moved from + # the container config to the host config (but are still accepted by + # create_container_config so warnings can be issued). + for arglist in (host_config_args, endpoint_config_args): + for item in arglist: + try: + config_args.remove(item) + except ValueError: + # Arg is not in config_args + pass + + return {'config': config_args, + 'host_config': host_config_args, + 'networking_config': endpoint_config_args} From cbd0270bac6d03bd037102cc3aa8afb556df2108 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Sat, 18 Feb 2017 19:47:36 -0600 Subject: [PATCH 12/12] docker: make docker-exec the default execution driver Docker 1.13.1 removed the ExecutionDriver from the ``docker info`` return data. This causes all attempts to run commands in containers to fall back to the old lxc-attach driver, which is incompatible with newer docker releases. --- salt/modules/dockerng.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/salt/modules/dockerng.py b/salt/modules/dockerng.py index fe077228db..bbcecfe90e 100644 --- a/salt/modules/dockerng.py +++ b/salt/modules/dockerng.py @@ -203,6 +203,14 @@ Functions Executing Commands Within a Running Container --------------------------------------------- +.. note:: + With the release of Docker 1.13.1, the Execution Driver has been removed. + Starting in versions 2016.3.6, 2016.11.4, and Nitrogen, Salt defaults to + using ``docker exec`` to run commands in containers, however for older Salt + releases it will be necessary to set the ``docker.exec_driver`` config + option to either ``docker-exec`` or ``nsenter`` for Docker versions 1.13.1 + and newer. + Multiple methods exist for executing commands within Docker containers: - lxc-attach_: Default for older versions of docker @@ -843,10 +851,12 @@ def _get_exec_driver(): __context__[contextkey] = from_config return from_config - # For old versions of docker, lxc was the only supported driver. - # This is a sane default. - driver = info().get('ExecutionDriver', 'lxc-') - if driver.startswith('lxc-'): + # The execution driver was removed in Docker 1.13.1, docker-exec is now + # the default. + driver = info().get('ExecutionDriver', 'docker-exec') + if driver == 'docker-exec': + __context__[contextkey] = driver + elif driver.startswith('lxc-'): __context__[contextkey] = 'lxc-attach' elif driver.startswith('native-') and HAS_NSENTER: __context__[contextkey] = 'nsenter' @@ -5510,9 +5520,7 @@ def get_client_args(): .. versionadded:: 2016.3.6,2016.11.4,Nitrogen Returns the args for docker-py's `low-level API`_, organized by container - config, host config, and networking config. This is designed for use by the - :mod:`docker states ` to more gracefully handle API - changes. + config, host config, and networking config. .. _`low-level API`: http://docker-py.readthedocs.io/en/stable/api.html