From a03335b3fe3f4d44b3f9400aa47ee53cf4b61c3c Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Wed, 12 Jul 2017 14:06:09 -0400 Subject: [PATCH 01/24] Added error logging for all VMware exceptions before raising Salt exceptions --- salt/utils/vmware.py | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/salt/utils/vmware.py b/salt/utils/vmware.py index 19292f2128..a405f5331a 100644 --- a/salt/utils/vmware.py +++ b/salt/utils/vmware.py @@ -242,8 +242,8 @@ def _get_service_instance(host, username, password, protocol, b64token=token, mechanism=mechanism) else: + log.error(exc) err_msg = exc.msg if hasattr(exc, 'msg') else default_msg - log.trace(exc) raise salt.exceptions.VMwareConnectionError(err_msg) except Exception as exc: if 'certificate verify failed' in str(exc): @@ -262,8 +262,8 @@ def _get_service_instance(host, username, password, protocol, mechanism=mechanism ) except Exception as exc: + log.error(exc) err_msg = exc.msg if hasattr(exc, 'msg') else str(exc) - log.trace(err_msg) raise salt.exceptions.VMwareConnectionError( 'Could not connect to host \'{0}\': ' '{1}'.format(host, err_msg)) @@ -389,8 +389,10 @@ def get_service_instance(host, username=None, password=None, protocol=None, principal, domain) except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) return service_instance @@ -426,8 +428,10 @@ def disconnect(service_instance): try: Disconnect(service_instance) except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) @@ -442,8 +446,10 @@ def is_connection_to_a_vcenter(service_instance): try: api_type = service_instance.content.about.apiType except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) log.trace('api_type = {0}'.format(api_type)) if api_type == 'VirtualCenter': @@ -649,8 +655,10 @@ def get_root_folder(service_instance): log.trace('Retrieving root folder') return service_instance.RetrieveContent().rootFolder except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) @@ -701,8 +709,10 @@ def get_content(service_instance, obj_type, property_list=None, obj_ref = service_instance.content.viewManager.CreateContainerView( container_ref, [obj_type], True) except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) # Create 'Traverse All' traversal spec to determine the path for @@ -739,8 +749,10 @@ def get_content(service_instance, obj_type, property_list=None, try: content = service_instance.content.propertyCollector.RetrieveContents([filter_spec]) except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) # Destroy the object view @@ -748,8 +760,10 @@ def get_content(service_instance, obj_type, property_list=None, try: obj_ref.Destroy() except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) return content @@ -1001,8 +1015,10 @@ def create_datacenter(service_instance, datacenter_name): try: dc_obj = root_folder.CreateDatacenter(datacenter_name) except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) return dc_obj @@ -1063,8 +1079,10 @@ def create_cluster(dc_ref, cluster_name, cluster_spec): try: dc_ref.hostFolder.CreateClusterEx(cluster_name, cluster_spec) except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) @@ -1085,8 +1103,10 @@ def update_cluster(cluster_ref, cluster_spec): task = cluster_ref.ReconfigureComputeResource_Task(cluster_spec, modify=True) except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) wait_for_task(task, cluster_name, 'ClusterUpdateTask') @@ -1292,8 +1312,10 @@ def wait_for_task(task, instance_name, task_type, sleep_seconds=1, log_level='de try: task_info = task.info except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) while task_info.state == 'running' or task_info.state == 'queued': if time_counter % sleep_seconds == 0: @@ -1308,8 +1330,10 @@ def wait_for_task(task, instance_name, task_type, sleep_seconds=1, log_level='de try: task_info = task.info except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: + log.error(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) if task_info.state == 'success': msg = '[ {0} ] Successfully completed {1} task in {2} seconds'.format( @@ -1325,10 +1349,13 @@ def wait_for_task(task, instance_name, task_type, sleep_seconds=1, log_level='de try: raise task_info.error except vim.fault.VimFault as exc: + log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.fault.SystemError as exc: + log.error(exc) raise salt.exceptions.VMwareSystemError(exc.msg) except vmodl.fault.InvalidArgument as exc: + log.error(exc) exc_message = exc.msg if exc.faultMessage: exc_message = '{0} ({1})'.format(exc_message, From b9ba62acaad07bf79c80d8bae2ee2df8a60d4c78 Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Thu, 13 Jul 2017 05:37:05 -0400 Subject: [PATCH 02/24] Caught vim.fault.NoPermission error in all utils.vmware functions --- salt/utils/vmware.py | 68 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/salt/utils/vmware.py b/salt/utils/vmware.py index a405f5331a..ddec069405 100644 --- a/salt/utils/vmware.py +++ b/salt/utils/vmware.py @@ -388,6 +388,11 @@ def get_service_instance(host, username=None, password=None, protocol=None, mechanism, principal, domain) + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -427,6 +432,11 @@ def disconnect(service_instance): log.trace('Disconnecting') try: Disconnect(service_instance) + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -445,6 +455,11 @@ def is_connection_to_a_vcenter(service_instance): ''' try: api_type = service_instance.content.about.apiType + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -654,6 +669,11 @@ def get_root_folder(service_instance): try: log.trace('Retrieving root folder') return service_instance.RetrieveContent().rootFolder + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -708,6 +728,11 @@ def get_content(service_instance, obj_type, property_list=None, try: obj_ref = service_instance.content.viewManager.CreateContainerView( container_ref, [obj_type], True) + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -748,6 +773,11 @@ def get_content(service_instance, obj_type, property_list=None, # Retrieve the contents try: content = service_instance.content.propertyCollector.RetrieveContents([filter_spec]) + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -759,6 +789,11 @@ def get_content(service_instance, obj_type, property_list=None, if local_traversal_spec: try: obj_ref.Destroy() + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -1014,6 +1049,11 @@ def create_datacenter(service_instance, datacenter_name): log.trace('Creating datacenter \'{0}\''.format(datacenter_name)) try: dc_obj = root_folder.CreateDatacenter(datacenter_name) + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -1022,7 +1062,6 @@ def create_datacenter(service_instance, datacenter_name): raise salt.exceptions.VMwareRuntimeError(exc.msg) return dc_obj - def get_cluster(dc_ref, cluster): ''' Returns a cluster in a datacenter. @@ -1078,6 +1117,11 @@ def create_cluster(dc_ref, cluster_name, cluster_spec): ''.format(cluster_name, dc_name)) try: dc_ref.hostFolder.CreateClusterEx(cluster_name, cluster_spec) + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -1102,6 +1146,11 @@ def update_cluster(cluster_ref, cluster_spec): try: task = cluster_ref.ReconfigureComputeResource_Task(cluster_spec, modify=True) + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -1311,6 +1360,11 @@ def wait_for_task(task, instance_name, task_type, sleep_seconds=1, log_level='de task.__class__.__name__)) try: task_info = task.info + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -1329,6 +1383,11 @@ def wait_for_task(task, instance_name, task_type, sleep_seconds=1, log_level='de time_counter += 1 try: task_info = task.info + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) @@ -1348,6 +1407,13 @@ def wait_for_task(task, instance_name, task_type, sleep_seconds=1, log_level='de # task is in an error state try: raise task_info.error + log.error(exc) + raise salt.exceptions.VMwareNotFoundError(exc.msg) + except vim.fault.NoPermission as exc: + log.error(exc) + raise salt.exceptions.VMwareApiError( + 'Not enough permissions. Required privilege: ' + '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: log.error(exc) raise salt.exceptions.VMwareApiError(exc.msg) From ec5532cdcfe1ef299cb79598ace0aeb985213107 Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Thu, 13 Jul 2017 05:57:57 -0400 Subject: [PATCH 03/24] Added no_permission tests to vmware functions --- tests/unit/utils/vmware_test/test_cluster.py | 23 ++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/unit/utils/vmware_test/test_cluster.py b/tests/unit/utils/vmware_test/test_cluster.py index daf157709c..9a628c9b36 100644 --- a/tests/unit/utils/vmware_test/test_cluster.py +++ b/tests/unit/utils/vmware_test/test_cluster.py @@ -178,6 +178,18 @@ class CreateClusterTestCase(TestCase): self.mock_create_cluster_ex.assert_called_once_with( 'fake_cluster', self.mock_cluster_spec) + def test_create_cluster_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + self.mock_dc.hostFolder.CreateClusterEx = MagicMock( + side_effect=exc) + with self.assertRaises(VMwareApiError) as excinfo: + vmware.create_cluster(self.mock_dc, 'fake_cluster', + self.mock_cluster_spec) + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_create_cluster_raise_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' @@ -234,6 +246,17 @@ class UpdateClusterTestCase(TestCase): self.mock_reconfigure_compute_resource_task.assert_called_once_with( self.mock_cluster_spec, modify=True) + def test_reconfigure_compute_resource_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + self.mock_cluster.ReconfigureComputeResource_Task = \ + MagicMock(side_effect=exc) + with self.assertRaises(VMwareApiError) as excinfo: + vmware.update_cluster(self.mock_cluster, self.mock_cluster_spec) + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_reconfigure_compute_resource_raise_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' From 62edc82d344ac4f0e6807080e3ced455cd77ea67 Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Thu, 13 Jul 2017 07:15:42 -0400 Subject: [PATCH 04/24] Added tests for catching vim.fault.NoPermission for all tested function in utils.vmware --- tests/unit/utils/vmware_test/test_common.py | 92 +++++++++++++++++++ .../unit/utils/vmware_test/test_connection.py | 42 +++++++++ .../unit/utils/vmware_test/test_datacenter.py | 13 +++ 3 files changed, 147 insertions(+) diff --git a/tests/unit/utils/vmware_test/test_common.py b/tests/unit/utils/vmware_test/test_common.py index 0de56813e5..76f77b2b14 100644 --- a/tests/unit/utils/vmware_test/test_common.py +++ b/tests/unit/utils/vmware_test/test_common.py @@ -42,6 +42,19 @@ class WaitForTaskTestCase(TestCase): patcher = patch(mod, mock) patcher.start() self.addCleanup(patcher.stop) + + def test_first_task_info_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + mock_task = MagicMock() + type(mock_task).info = PropertyMock(side_effect=exc) + with self.assertRaises(excs.VMwareApiError) as excinfo: + salt.utils.vmware.wait_for_task(mock_task, + 'fake_instance_name', + 'task_type') + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') def test_first_task_info_raise_vim_fault(self): exc = vim.fault.VimFault() @@ -65,6 +78,22 @@ class WaitForTaskTestCase(TestCase): 'task_type') self.assertEqual(excinfo.exception.strerror, 'RuntimeFault msg') + def test_inner_loop_task_info_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + mock_task = MagicMock() + mock_info1 = MagicMock() + type(mock_task).info = PropertyMock( + side_effect=[mock_info1, exc]) + type(mock_info1).state = PropertyMock(side_effect=['running', 'bad']) + with self.assertRaises(excs.VMwareApiError) as excinfo: + salt.utils.vmware.wait_for_task(mock_task, + 'fake_instance_name', + 'task_type') + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_inner_loop_task_info_raise_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' @@ -161,6 +190,22 @@ class WaitForTaskTestCase(TestCase): 'task_type') self.assertEqual(str(excinfo.exception), 'error exc') + def test_info_error_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + mock_task = MagicMock() + prop_mock_state = PropertyMock(return_value='error') + prop_mock_error = PropertyMock(side_effect=exc) + type(mock_task.info).state = prop_mock_state + type(mock_task.info).error = prop_mock_error + with self.assertRaises(excs.VMwareApiError) as excinfo: + salt.utils.vmware.wait_for_task(mock_task, + 'fake_instance_name', + 'task_type') + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_info_error_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' @@ -658,6 +703,19 @@ class GetContentTestCase(TestCase): # check destroy is called self.assertEqual(self.destroy_mock.call_count, 1) + def test_create_container_view_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + self.si_mock.content.viewManager.CreateContainerView = \ + MagicMock(side_effect=exc) + with patch('salt.utils.vmware.get_root_folder', + self.get_root_folder_mock): + with self.assertRaises(excs.VMwareApiError) as excinfo: + salt.utils.vmware.get_content(self.si_mock, self.obj_type_mock) + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_create_container_view_raise_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' @@ -680,6 +738,19 @@ class GetContentTestCase(TestCase): salt.utils.vmware.get_content(self.si_mock, self.obj_type_mock) self.assertEqual(excinfo.exception.strerror, 'RuntimeFault msg') + def test_destroy_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + self.si_mock.content.viewManager.CreateContainerView = MagicMock( + return_value=MagicMock(Destroy=MagicMock(side_effect=exc))) + with patch('salt.utils.vmware.get_root_folder', + self.get_root_folder_mock): + with self.assertRaises(excs.VMwareApiError) as excinfo: + salt.utils.vmware.get_content(self.si_mock, self.obj_type_mock) + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_destroy_raise_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' @@ -745,6 +816,17 @@ class GetContentTestCase(TestCase): [self.filter_spec_ret_mock]) self.assertEqual(ret, self.result_mock) + def test_retrieve_contents_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + self.si_mock.content.propertyCollector.RetrieveContents = \ + MagicMock(side_effect=exc) + with self.assertRaises(excs.VMwareApiError) as excinfo: + salt.utils.vmware.get_content(self.si_mock, self.obj_type_mock) + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_retrieve_contents_raise_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' @@ -789,6 +871,16 @@ class GetRootFolderTestCase(TestCase): self.mock_si = MagicMock( RetrieveContent=MagicMock(return_value=self.mock_content)) + def test_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + type(self.mock_content).rootFolder = PropertyMock(side_effect=exc) + with self.assertRaises(excs.VMwareApiError) as excinfo: + salt.utils.vmware.get_root_folder(self.mock_si) + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_raise_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' diff --git a/tests/unit/utils/vmware_test/test_connection.py b/tests/unit/utils/vmware_test/test_connection.py index 812cb1231c..ff00a5f94e 100644 --- a/tests/unit/utils/vmware_test/test_connection.py +++ b/tests/unit/utils/vmware_test/test_connection.py @@ -688,6 +688,26 @@ class GetServiceInstanceTestCase(TestCase): self.assertEqual(mock_disconnect.call_count, 1) self.assertEqual(mock_get_si.call_count, 2) + def test_current_time_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + with patch('salt.utils.vmware._get_service_instance', + MagicMock(return_value=MagicMock( + CurrentTime=MagicMock(side_effect=exc)))): + with self.assertRaises(excs.VMwareApiError) as excinfo: + salt.utils.vmware.get_service_instance( + host='fake_host', + username='fake_username', + password='fake_password', + protocol='fake_protocol', + port=1, + mechanism='fake_mechanism', + principal='fake_principal', + domain='fake_domain') + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_current_time_raise_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' @@ -741,6 +761,17 @@ class DisconnectTestCase(TestCase): service_instance=self.mock_si) mock_disconnect.assert_called_once_with(self.mock_si) + def test_disconnect_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + with patch('salt.utils.vmware.Disconnect', MagicMock(side_effect=exc)): + with self.assertRaises(excs.VMwareApiError) as excinfo: + salt.utils.vmware.disconnect( + service_instance=self.mock_si) + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_disconnect_raise_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' @@ -765,6 +796,17 @@ class DisconnectTestCase(TestCase): class IsConnectionToAVCenterTestCase(TestCase): '''Tests for salt.utils.vmware.is_connection_to_a_vcenter''' + def test_api_type_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + mock_si = MagicMock() + type(mock_si.content.about).apiType = PropertyMock(side_effect=exc) + with self.assertRaises(excs.VMwareApiError) as excinfo: + salt.utils.vmware.is_connection_to_a_vcenter(mock_si) + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_api_type_raise_vim_fault(self): exc = vim.fault.VimFault() exc.msg = 'VimFault msg' diff --git a/tests/unit/utils/vmware_test/test_datacenter.py b/tests/unit/utils/vmware_test/test_datacenter.py index 86d1f0995a..c8045fb566 100644 --- a/tests/unit/utils/vmware_test/test_datacenter.py +++ b/tests/unit/utils/vmware_test/test_datacenter.py @@ -164,6 +164,19 @@ class CreateDatacenterTestCase(TestCase): vmware.create_datacenter(self.mock_si, 'fake_dc') self.mock_create_datacenter.assert_called_once_with('fake_dc') + def test_create_datacenter_raise_no_permission(self): + exc = vim.fault.NoPermission() + exc.privilegeId = 'Fake privilege' + self.mock_root_folder = MagicMock( + CreateDatacenter=MagicMock(side_effect=exc)) + with patch('salt.utils.vmware.get_root_folder', + MagicMock(return_value=self.mock_root_folder)): + with self.assertRaises(VMwareApiError) as excinfo: + vmware.create_datacenter(self.mock_si, 'fake_dc') + self.assertEqual(excinfo.exception.strerror, + 'Not enough permissions. Required privilege: ' + 'Fake privilege') + def test_create_datacenter_raise_vim_fault(self): exc = vim.VimFault() exc.msg = 'VimFault msg' From c256eaef8243597ca360a4453388085dec888497 Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Thu, 13 Jul 2017 11:00:52 -0400 Subject: [PATCH 05/24] Renamed vmware_test folder to vmware to follow new naming standard --- tests/unit/utils/{vmware_test => vmware}/__init__.py | 0 tests/unit/utils/{vmware_test => vmware}/test_cluster.py | 0 tests/unit/utils/{vmware_test => vmware}/test_common.py | 0 tests/unit/utils/{vmware_test => vmware}/test_connection.py | 0 tests/unit/utils/{vmware_test => vmware}/test_datacenter.py | 0 tests/unit/utils/{vmware_test => vmware}/test_host.py | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename tests/unit/utils/{vmware_test => vmware}/__init__.py (100%) rename tests/unit/utils/{vmware_test => vmware}/test_cluster.py (100%) rename tests/unit/utils/{vmware_test => vmware}/test_common.py (100%) rename tests/unit/utils/{vmware_test => vmware}/test_connection.py (100%) rename tests/unit/utils/{vmware_test => vmware}/test_datacenter.py (100%) rename tests/unit/utils/{vmware_test => vmware}/test_host.py (100%) diff --git a/tests/unit/utils/vmware_test/__init__.py b/tests/unit/utils/vmware/__init__.py similarity index 100% rename from tests/unit/utils/vmware_test/__init__.py rename to tests/unit/utils/vmware/__init__.py diff --git a/tests/unit/utils/vmware_test/test_cluster.py b/tests/unit/utils/vmware/test_cluster.py similarity index 100% rename from tests/unit/utils/vmware_test/test_cluster.py rename to tests/unit/utils/vmware/test_cluster.py diff --git a/tests/unit/utils/vmware_test/test_common.py b/tests/unit/utils/vmware/test_common.py similarity index 100% rename from tests/unit/utils/vmware_test/test_common.py rename to tests/unit/utils/vmware/test_common.py diff --git a/tests/unit/utils/vmware_test/test_connection.py b/tests/unit/utils/vmware/test_connection.py similarity index 100% rename from tests/unit/utils/vmware_test/test_connection.py rename to tests/unit/utils/vmware/test_connection.py diff --git a/tests/unit/utils/vmware_test/test_datacenter.py b/tests/unit/utils/vmware/test_datacenter.py similarity index 100% rename from tests/unit/utils/vmware_test/test_datacenter.py rename to tests/unit/utils/vmware/test_datacenter.py diff --git a/tests/unit/utils/vmware_test/test_host.py b/tests/unit/utils/vmware/test_host.py similarity index 100% rename from tests/unit/utils/vmware_test/test_host.py rename to tests/unit/utils/vmware/test_host.py From 785b9876a24b88e852fd930712e742be1e8930b9 Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Tue, 18 Jul 2017 05:17:44 -0400 Subject: [PATCH 06/24] Removed additional raise in vmware.wait_for_task --- salt/utils/vmware.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/salt/utils/vmware.py b/salt/utils/vmware.py index ddec069405..323dc58ded 100644 --- a/salt/utils/vmware.py +++ b/salt/utils/vmware.py @@ -1407,8 +1407,6 @@ def wait_for_task(task, instance_name, task_type, sleep_seconds=1, log_level='de # task is in an error state try: raise task_info.error - log.error(exc) - raise salt.exceptions.VMwareNotFoundError(exc.msg) except vim.fault.NoPermission as exc: log.error(exc) raise salt.exceptions.VMwareApiError( From 1f35aff238311043c4fbb6816c692fab4fec075d Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Tue, 18 Jul 2017 05:26:51 -0400 Subject: [PATCH 07/24] Added new line removed by mistake --- salt/utils/vmware.py | 1 + 1 file changed, 1 insertion(+) diff --git a/salt/utils/vmware.py b/salt/utils/vmware.py index 323dc58ded..e7b262b321 100644 --- a/salt/utils/vmware.py +++ b/salt/utils/vmware.py @@ -1062,6 +1062,7 @@ def create_datacenter(service_instance, datacenter_name): raise salt.exceptions.VMwareRuntimeError(exc.msg) return dc_obj + def get_cluster(dc_ref, cluster): ''' Returns a cluster in a datacenter. From e783d6edd53bb25cd21c9d72027d8d15c6936ad4 Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Tue, 18 Jul 2017 06:44:34 -0400 Subject: [PATCH 08/24] pylint --- tests/unit/utils/vmware/test_common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/utils/vmware/test_common.py b/tests/unit/utils/vmware/test_common.py index 76f77b2b14..fd50ed12d0 100644 --- a/tests/unit/utils/vmware/test_common.py +++ b/tests/unit/utils/vmware/test_common.py @@ -42,7 +42,7 @@ class WaitForTaskTestCase(TestCase): patcher = patch(mod, mock) patcher.start() self.addCleanup(patcher.stop) - + def test_first_task_info_raise_no_permission(self): exc = vim.fault.NoPermission() exc.privilegeId = 'Fake privilege' From 08c8a17746afbf8f1d821fa77a4a59be93ff4b7d Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Wed, 19 Jul 2017 07:07:44 -0400 Subject: [PATCH 09/24] Replaced log.error with log.exception in utils.vmware --- salt/utils/vmware.py | 84 ++++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/salt/utils/vmware.py b/salt/utils/vmware.py index e7b262b321..dfc4adbb5a 100644 --- a/salt/utils/vmware.py +++ b/salt/utils/vmware.py @@ -242,7 +242,7 @@ def _get_service_instance(host, username, password, protocol, b64token=token, mechanism=mechanism) else: - log.error(exc) + log.exception(exc) err_msg = exc.msg if hasattr(exc, 'msg') else default_msg raise salt.exceptions.VMwareConnectionError(err_msg) except Exception as exc: @@ -262,7 +262,7 @@ def _get_service_instance(host, username, password, protocol, mechanism=mechanism ) except Exception as exc: - log.error(exc) + log.exception(exc) err_msg = exc.msg if hasattr(exc, 'msg') else str(exc) raise salt.exceptions.VMwareConnectionError( 'Could not connect to host \'{0}\': ' @@ -389,15 +389,15 @@ def get_service_instance(host, username=None, password=None, protocol=None, principal, domain) except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) return service_instance @@ -433,15 +433,15 @@ def disconnect(service_instance): try: Disconnect(service_instance) except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) @@ -456,15 +456,15 @@ def is_connection_to_a_vcenter(service_instance): try: api_type = service_instance.content.about.apiType except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) log.trace('api_type = {0}'.format(api_type)) if api_type == 'VirtualCenter': @@ -670,15 +670,15 @@ def get_root_folder(service_instance): log.trace('Retrieving root folder') return service_instance.RetrieveContent().rootFolder except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) @@ -729,15 +729,15 @@ def get_content(service_instance, obj_type, property_list=None, obj_ref = service_instance.content.viewManager.CreateContainerView( container_ref, [obj_type], True) except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) # Create 'Traverse All' traversal spec to determine the path for @@ -774,15 +774,15 @@ def get_content(service_instance, obj_type, property_list=None, try: content = service_instance.content.propertyCollector.RetrieveContents([filter_spec]) except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) # Destroy the object view @@ -790,15 +790,15 @@ def get_content(service_instance, obj_type, property_list=None, try: obj_ref.Destroy() except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) return content @@ -1050,15 +1050,15 @@ def create_datacenter(service_instance, datacenter_name): try: dc_obj = root_folder.CreateDatacenter(datacenter_name) except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) return dc_obj @@ -1119,15 +1119,15 @@ def create_cluster(dc_ref, cluster_name, cluster_spec): try: dc_ref.hostFolder.CreateClusterEx(cluster_name, cluster_spec) except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) @@ -1148,15 +1148,15 @@ def update_cluster(cluster_ref, cluster_spec): task = cluster_ref.ReconfigureComputeResource_Task(cluster_spec, modify=True) except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) wait_for_task(task, cluster_name, 'ClusterUpdateTask') @@ -1362,15 +1362,15 @@ def wait_for_task(task, instance_name, task_type, sleep_seconds=1, log_level='de try: task_info = task.info except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) while task_info.state == 'running' or task_info.state == 'queued': if time_counter % sleep_seconds == 0: @@ -1385,15 +1385,15 @@ def wait_for_task(task, instance_name, task_type, sleep_seconds=1, log_level='de try: task_info = task.info except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.RuntimeFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareRuntimeError(exc.msg) if task_info.state == 'success': msg = '[ {0} ] Successfully completed {1} task in {2} seconds'.format( @@ -1409,18 +1409,18 @@ def wait_for_task(task, instance_name, task_type, sleep_seconds=1, log_level='de try: raise task_info.error except vim.fault.NoPermission as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError( 'Not enough permissions. Required privilege: ' '{}'.format(exc.privilegeId)) except vim.fault.VimFault as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareApiError(exc.msg) except vmodl.fault.SystemError as exc: - log.error(exc) + log.exception(exc) raise salt.exceptions.VMwareSystemError(exc.msg) except vmodl.fault.InvalidArgument as exc: - log.error(exc) + log.exception(exc) exc_message = exc.msg if exc.faultMessage: exc_message = '{0} ({1})'.format(exc_message, From 6194842eb85db7cc1ad835698943f7b78ae6e094 Mon Sep 17 00:00:00 2001 From: Michael Gibson Date: Thu, 20 Jul 2017 10:41:39 -0600 Subject: [PATCH 10/24] adding 2-factor auth capability to ldap eauth module - #42280 --- salt/auth/ldap.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/salt/auth/ldap.py b/salt/auth/ldap.py index 396c1d00a2..7da2a62a0b 100644 --- a/salt/auth/ldap.py +++ b/salt/auth/ldap.py @@ -280,8 +280,14 @@ def auth(username, password): ''' Simple LDAP auth ''' - if _bind(username, password, anonymous=_config('auth_by_group_membership_only', mandatory=False) and - _config('anonymous', mandatory=False)): + #If bind credentials are configured, use them instead of user's + if _config('binddn', mandatory=False) and _config('bindpw', mandatory=False): + bind = _bind_for_search(anonymous=_config('anonymous', mandatory=False)) + else: + bind = _bind(username, password, anonymous=_config('auth_by_group_membership_only', mandatory=False) and + _config('anonymous', mandatory=False)) + + if bind: log.debug('LDAP authentication successful') return True else: @@ -306,8 +312,9 @@ def groups(username, **kwargs): ''' group_list = [] - bind = _bind(username, kwargs['password'], - anonymous=_config('anonymous', mandatory=False)) + # Perform un-authenticated bind to determine group membership + bind = _bind_for_search(anonymous=_config('anonymous', mandatory=False)) + if bind: log.debug('ldap bind to determine group membership succeeded!') @@ -381,7 +388,11 @@ def groups(username, **kwargs): group_list.append(group.split(',')[0].split('=')[-1]) log.debug('User {0} is a member of groups: {1}'.format(username, group_list)) - if not auth(username, kwargs['password']): + # Only test user auth on first call for job. + # 'show_jid' only exists on first payload so we can use that for the conditional. + if 'show_jid' in kwargs and not _bind(username, kwargs['password'], + anonymous=_config('auth_by_group_membership_only', mandatory=False) and + _config('anonymous', mandatory=False)): log.error('LDAP username and password do not match') return [] else: From 2bae2b149bcebb61991f430b3c553d515a2eae58 Mon Sep 17 00:00:00 2001 From: Sayyid Hamid Mahdavi Date: Sun, 23 Jul 2017 15:53:52 +0430 Subject: [PATCH 11/24] remove .encode(__salt_system_encoding__)) .encode(__salt_system_encoding__)) not necessary in python3 --- salt/master.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/salt/master.py b/salt/master.py index a6b292d458..51acb5962a 100644 --- a/salt/master.py +++ b/salt/master.py @@ -1305,10 +1305,8 @@ class AESFuncs(object): with salt.utils.files.fopen(cpath, mode) as fp_: if load['loc']: fp_.seek(load['loc']) - if six.PY3: - fp_.write(load['data'].encode(__salt_system_encoding__)) - else: - fp_.write(load['data']) + + fp_.write(load['data']) return True def _pillar(self, load): From ea38b4c6fa186913b122aa930efb5f00923fc60c Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Thu, 6 Jul 2017 14:06:23 +0100 Subject: [PATCH 12/24] Improved comment and added changes to the results of applying the linux_acl.present state --- salt/states/linux_acl.py | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/salt/states/linux_acl.py b/salt/states/linux_acl.py index a6a54a7fcd..7a468970d6 100644 --- a/salt/states/linux_acl.py +++ b/salt/states/linux_acl.py @@ -99,21 +99,43 @@ def present(name, acl_type, acl_name='', perms='', recurse=False): if user[_search_name]['octal'] == sum([_octal.get(i, i) for i in perms]): ret['comment'] = 'Permissions are in the desired state' else: - ret['comment'] = 'Permissions have been updated' + changes = {'new': {'acl_name': acl_name, + 'acl_type': acl_type, + 'perms': perms}, + 'old': {'acl_name': acl_name, + 'acl_type': acl_type, + 'perms': str(user[_search_name]['octal'])}} if __opts__['test']: - ret['result'] = None + ret.update({'comment': 'Updated permissions will be applied for ' + '{0}: {1} -> {2}'.format( + acl_name, + str(user[_search_name]['octal']), + perms), + 'result': None, 'pchanges': changes}) return ret __salt__['acl.modfacl'](acl_type, acl_name, perms, name, recursive=recurse) + ret.update({'comment': 'Updated permissions for ' + '{0}'.format(acl_name), + 'result': True, 'changes': changes}) else: - ret['comment'] = 'Permissions will be applied' + changes = {'new': {'acl_name': acl_name, + 'acl_type': acl_type, + 'perms': perms}} if __opts__['test']: + ret.update({'comment': 'New permissions will be applied for ' + '{0}: {1}'.format(acl_name, perms), + 'result': None, 'pchanges': changes}) ret['result'] = None return ret __salt__['acl.modfacl'](acl_type, acl_name, perms, name, recursive=recurse) + ret.update({'comment': 'Updated permissions for ' + '{0}'.format(acl_name), + 'result': True, 'changes': changes}) + else: ret['comment'] = 'ACL Type does not exist' ret['result'] = False From 4d5aedca623b243fad7537d7fe9eb617c221e380 Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Mon, 10 Jul 2017 11:28:26 +0100 Subject: [PATCH 13/24] Added option to raise a CommandExecutionError if an error is encountered when invoking cmd.run --- salt/modules/cmdmod.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index a864e8a7be..920cdffd1a 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -770,6 +770,7 @@ def run(cmd, bg=False, password=None, encoded_cmd=False, + raise_err=False, **kwargs): r''' Execute the passed command and return the output as a string @@ -873,6 +874,9 @@ def run(cmd, :param bool encoded_cmd: Specify if the supplied command is encoded. Only applies to shell 'powershell'. + :pram bool raise_err: Specifies whether to raise a CommandExecutionError. + If False, the error will be logged. Default is False. + .. warning:: This function does not process commands through a shell unless the python_shell flag is set to True. This means that any @@ -962,6 +966,8 @@ def run(cmd, ) ) log.error(log_callback(msg)) + if raise_err: + raise CommandExecutionError(log_callback(ret['stdout'])) log.log(lvl, 'output: {0}'.format(log_callback(ret['stdout']))) return ret['stdout'] From 17447bf8c9d01fa75ded1b5c8e2289eba246cf13 Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Mon, 10 Jul 2017 11:29:46 +0100 Subject: [PATCH 14/24] Updated acl.modfacl to raise any error encountered --- salt/modules/linux_acl.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/salt/modules/linux_acl.py b/salt/modules/linux_acl.py index a7fa3cbd1c..ca1b225c91 100644 --- a/salt/modules/linux_acl.py +++ b/salt/modules/linux_acl.py @@ -213,8 +213,10 @@ def modfacl(acl_type, acl_name='', perms='', *args, **kwargs): salt '*' acl.modfacl d:u myuser 7 /tmp/house/kitchen salt '*' acl.modfacl g mygroup 0 /tmp/house/kitchen /tmp/house/livingroom salt '*' acl.modfacl user myuser rwx /tmp/house/kitchen recursive=True + salt '*' acl.modfacl user myuser rwx /tmp/house/kitchen raise_err=True ''' recursive = kwargs.pop('recursive', False) + raise_err = kwargs.pop('raise_err', False) _raise_on_no_files(*args) @@ -228,7 +230,7 @@ def modfacl(acl_type, acl_name='', perms='', *args, **kwargs): for dentry in args: cmd += ' "{0}"'.format(dentry) - __salt__['cmd.run'](cmd, python_shell=False) + __salt__['cmd.run'](cmd, python_shell=False, raise_err=raise_err) return True From 0350609a3aa42144c893d758355e5d96e433d9ef Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Mon, 10 Jul 2017 11:30:51 +0100 Subject: [PATCH 15/24] Catch errors raised by acl.modfacl in linux_acl.present state --- salt/states/linux_acl.py | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/salt/states/linux_acl.py b/salt/states/linux_acl.py index 7a468970d6..b3d6728468 100644 --- a/salt/states/linux_acl.py +++ b/salt/states/linux_acl.py @@ -34,6 +34,9 @@ import os # Import salt libs import salt.utils +# Impot salt exceptions +from salt.exceptions import CommandExecutionError + # Import 3rd-party libs import salt.ext.six as six @@ -57,6 +60,7 @@ def present(name, acl_type, acl_name='', perms='', recurse=False): ret = {'name': name, 'result': True, 'changes': {}, + 'pchanges': {}, 'comment': ''} _octal = {'r': 4, 'w': 2, 'x': 1, '-': 0} @@ -114,11 +118,16 @@ def present(name, acl_type, acl_name='', perms='', recurse=False): perms), 'result': None, 'pchanges': changes}) return ret - - __salt__['acl.modfacl'](acl_type, acl_name, perms, name, recursive=recurse) - ret.update({'comment': 'Updated permissions for ' - '{0}'.format(acl_name), - 'result': True, 'changes': changes}) + try: + __salt__['acl.modfacl'](acl_type, acl_name, perms, name, + recursive=recurse, raise_err=True) + ret.update({'comment': 'Updated permissions for ' + '{0}'.format(acl_name), + 'result': True, 'changes': changes}) + except CommandExecutionError as exc: + ret.update({'comment': 'Error updating permissions for ' + '{0}: {1}'.format(acl_name, exc.strerror), + 'result': False}) else: changes = {'new': {'acl_name': acl_name, 'acl_type': acl_type, @@ -131,10 +140,16 @@ def present(name, acl_type, acl_name='', perms='', recurse=False): ret['result'] = None return ret - __salt__['acl.modfacl'](acl_type, acl_name, perms, name, recursive=recurse) - ret.update({'comment': 'Updated permissions for ' - '{0}'.format(acl_name), - 'result': True, 'changes': changes}) + try: + __salt__['acl.modfacl'](acl_type, acl_name, perms, name, + recursive=recurse, raise_err=True) + ret.update({'comment': 'Applied new permissions for ' + '{0}'.format(acl_name), + 'result': True, 'changes': changes}) + except CommandExecutionError as exc: + ret.update({'comment': 'Error updating permissions for {0}: ' + '{1}'.format(acl_name, exc.strerror), + 'result': False}) else: ret['comment'] = 'ACL Type does not exist' From 8e415c8ef7b5f23d8dd49acfb6dae194803dba1c Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Mon, 10 Jul 2017 16:40:02 +0100 Subject: [PATCH 16/24] Updated and added linux_acl_test.present tests --- tests/unit/states/test_linux_acl.py | 118 ++++++++++++++++++++++++---- 1 file changed, 103 insertions(+), 15 deletions(-) diff --git a/tests/unit/states/test_linux_acl.py b/tests/unit/states/test_linux_acl.py index 3ade701b82..f5488d2ae4 100644 --- a/tests/unit/states/test_linux_acl.py +++ b/tests/unit/states/test_linux_acl.py @@ -15,8 +15,10 @@ from tests.support.mock import ( MagicMock, patch) + # Import Salt Libs import salt.states.linux_acl as linux_acl +from salt.exceptions import CommandExecutionError @skipIf(NO_MOCK, NO_MOCK_REASON) @@ -34,34 +36,120 @@ class LinuxAclTestCase(TestCase, LoaderModuleMockMixin): ''' Test to ensure a Linux ACL is present ''' + self.maxDiff = None name = '/root' acl_type = 'users' acl_name = 'damian' perms = 'rwx' - ret = {'name': name, - 'result': None, - 'comment': '', - 'changes': {}} - mock = MagicMock(side_effect=[{name: {acl_type: [{acl_name: - {'octal': 'A'}}]}}, + {'octal': 'A'}}]}}, + {name: {acl_type: [{acl_name: + {'octal': 'A'}}]}}, + {name: {acl_type: [{acl_name: + {'octal': 'A'}}]}}, + {name: {acl_type: [{}]}}, + {name: {acl_type: [{}]}}, {name: {acl_type: [{}]}}, {name: {acl_type: ''}}]) + mock_modfacl = MagicMock(return_value=True) + with patch.dict(linux_acl.__salt__, {'acl.getfacl': mock}): + # Update - test=True with patch.dict(linux_acl.__opts__, {'test': True}): - comt = ('Permissions have been updated') - ret.update({'comment': comt}) + comt = ('Updated permissions will be applied for {0}: A -> {1}' + ''.format(acl_name, perms)) + ret = {'name': name, + 'comment': comt, + 'changes': {}, + 'pchanges': {'new': {'acl_name': acl_name, + 'acl_type': acl_type, + 'perms': perms}, + 'old': {'acl_name': acl_name, + 'acl_type': acl_type, + 'perms': 'A'}}, + 'result': None} + self.assertDictEqual(linux_acl.present(name, acl_type, acl_name, perms), ret) - - comt = ('Permissions will be applied') - ret.update({'comment': comt}) - self.assertDictEqual(linux_acl.present(name, acl_type, acl_name, - perms), ret) - + # Update - test=False + with patch.dict(linux_acl.__salt__, {'acl.modfacl': mock_modfacl}): + with patch.dict(linux_acl.__opts__, {'test': False}): + comt = ('Updated permissions for {0}'.format(acl_name)) + ret = {'name': name, + 'comment': comt, + 'changes': {'new': {'acl_name': acl_name, + 'acl_type': acl_type, + 'perms': perms}, + 'old': {'acl_name': acl_name, + 'acl_type': acl_type, + 'perms': 'A'}}, + 'pchanges': {}, + 'result': True} + self.assertDictEqual(linux_acl.present(name, acl_type, + acl_name, perms), + ret) + # Update - modfacl error + with patch.dict(linux_acl.__salt__, {'acl.modfacl': MagicMock( + side_effect=CommandExecutionError('Custom err'))}): + with patch.dict(linux_acl.__opts__, {'test': False}): + comt = ('Error updating permissions for {0}: Custom err' + ''.format(acl_name)) + ret = {'name': name, + 'comment': comt, + 'changes': {}, + 'pchanges': {}, + 'result': False} + self.assertDictEqual(linux_acl.present(name, acl_type, + acl_name, perms), + ret) + # New - test=True + with patch.dict(linux_acl.__salt__, {'acl.modfacl': mock_modfacl}): + with patch.dict(linux_acl.__opts__, {'test': True}): + comt = ('New permissions will be applied ' + 'for {0}: {1}'.format(acl_name, perms)) + ret = {'name': name, + 'comment': comt, + 'changes': {}, + 'pchanges': {'new': {'acl_name': acl_name, + 'acl_type': acl_type, + 'perms': perms}}, + 'result': None} + self.assertDictEqual(linux_acl.present(name, acl_type, + acl_name, perms), + ret) + # New - test=False + with patch.dict(linux_acl.__salt__, {'acl.modfacl': mock_modfacl}): + with patch.dict(linux_acl.__opts__, {'test': False}): + comt = ('Applied new permissions for {0}'.format(acl_name)) + ret = {'name': name, + 'comment': comt, + 'changes': {'new': {'acl_name': acl_name, + 'acl_type': acl_type, + 'perms': perms}}, + 'pchanges': {}, + 'result': True} + self.assertDictEqual(linux_acl.present(name, acl_type, + acl_name, perms), + ret) + # New - modfacl error + with patch.dict(linux_acl.__salt__, {'acl.modfacl': MagicMock( + side_effect=CommandExecutionError('Custom err'))}): + with patch.dict(linux_acl.__opts__, {'test': False}): + comt = ('Error updating permissions for {0}: Custom err' + ''.format(acl_name)) + ret = {'name': name, + 'comment': comt, + 'changes': {}, + 'pchanges': {}, + 'result': False} + self.assertDictEqual(linux_acl.present(name, acl_type, + acl_name, perms), + ret) + # No acl type comt = ('ACL Type does not exist') - ret.update({'comment': comt, 'result': False}) + ret = {'name': name, 'comment': comt, 'result': False, + 'changes': {}, 'pchanges': {}} self.assertDictEqual(linux_acl.present(name, acl_type, acl_name, perms), ret) From 0f3d784c0f4f8153b0250af984273e2a445899e6 Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Mon, 10 Jul 2017 17:36:42 +0100 Subject: [PATCH 17/24] Updated all tests for modules.mod_acl and added one when the error is raised --- tests/unit/modules/test_linux_acl.py | 39 ++++++++++++++++------------ 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/tests/unit/modules/test_linux_acl.py b/tests/unit/modules/test_linux_acl.py index 9fc07569be..429f45190d 100644 --- a/tests/unit/modules/test_linux_acl.py +++ b/tests/unit/modules/test_linux_acl.py @@ -6,7 +6,7 @@ from __future__ import absolute_import # Import Salt Testing libs from tests.support.mixins import LoaderModuleMockMixin from tests.support.unit import skipIf, TestCase -from tests.support.mock import NO_MOCK, NO_MOCK_REASON, MagicMock +from tests.support.mock import NO_MOCK, NO_MOCK_REASON, MagicMock, patch # Import salt libs import salt.modules.linux_acl as linux_acl @@ -84,63 +84,70 @@ class LinuxAclTestCase(TestCase, LoaderModuleMockMixin): def test_modfacl__u_w_single_arg(self): linux_acl.modfacl(*(self.u_acl + [self.file])) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.user_acl_cmd, self.quoted_file]), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.user_acl_cmd, self.quoted_file]), python_shell=False, raise_err=False) def test_modfacl__u_w_multiple_args(self): linux_acl.modfacl(*(self.u_acl + self.files)) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.user_acl_cmd] + self.quoted_files), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.user_acl_cmd] + self.quoted_files), python_shell=False, raise_err=False) def test_modfacl__user_w_single_arg(self): linux_acl.modfacl(*(self.user_acl + [self.file])) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.user_acl_cmd, self.quoted_file]), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.user_acl_cmd, self.quoted_file]), python_shell=False, raise_err=False) def test_modfacl__user_w_multiple_args(self): linux_acl.modfacl(*(self.user_acl + self.files)) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.user_acl_cmd] + self.quoted_files), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.user_acl_cmd] + self.quoted_files), python_shell=False, raise_err=False) def test_modfacl__g_w_single_arg(self): linux_acl.modfacl(*(self.g_acl + [self.file])) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.group_acl_cmd, self.quoted_file]), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.group_acl_cmd, self.quoted_file]), python_shell=False, raise_err=False) def test_modfacl__g_w_multiple_args(self): linux_acl.modfacl(*(self.g_acl + self.files)) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.group_acl_cmd] + self.quoted_files), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.group_acl_cmd] + self.quoted_files), python_shell=False, raise_err=False) def test_modfacl__group_w_single_arg(self): linux_acl.modfacl(*(self.group_acl + [self.file])) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.group_acl_cmd, self.quoted_file]), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.group_acl_cmd, self.quoted_file]), python_shell=False, raise_err=False) def test_modfacl__group_w_multiple_args(self): linux_acl.modfacl(*(self.group_acl + self.files)) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.group_acl_cmd] + self.quoted_files), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.group_acl_cmd] + self.quoted_files), python_shell=False, raise_err=False) def test_modfacl__d_u_w_single_arg(self): linux_acl.modfacl(*(self.d_u_acl + [self.file])) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd, self.quoted_file]), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd, self.quoted_file]), python_shell=False, raise_err=False) def test_modfacl__d_u_w_multiple_args(self): linux_acl.modfacl(*(self.d_u_acl + self.files)) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd] + self.quoted_files), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd] + self.quoted_files), python_shell=False, raise_err=False) def test_modfacl__d_user_w_single_arg(self): linux_acl.modfacl(*(self.d_user_acl + [self.file])) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd, self.quoted_file]), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd, self.quoted_file]), python_shell=False, raise_err=False) def test_modfacl__d_user_w_multiple_args(self): linux_acl.modfacl(*(self.d_user_acl + self.files)) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd] + self.quoted_files), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd] + self.quoted_files), python_shell=False, raise_err=False) def test_modfacl__default_user_w_single_arg(self): linux_acl.modfacl(*(self.default_user_acl + [self.file])) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd, self.quoted_file]), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd, self.quoted_file]), python_shell=False, raise_err=False) def test_modfacl__default_user_w_multiple_args(self): linux_acl.modfacl(*(self.default_user_acl + self.files)) - self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd] + self.quoted_files), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -m ' + ' '.join([self.default_user_acl_cmd] + self.quoted_files), python_shell=False, raise_err=False) def test_modfacl__recursive_w_multiple_args(self): linux_acl.modfacl(*(self.user_acl + self.files), recursive=True) - self.cmdrun.assert_called_once_with('setfacl -R -m ' + ' '.join([self.user_acl_cmd] + self.quoted_files), python_shell=False) + self.cmdrun.assert_called_once_with('setfacl -R -m ' + ' '.join([self.user_acl_cmd] + self.quoted_files), python_shell=False, raise_err=False) + + def test_modfacl_raise_err(self): + mock = MagicMock(side_effect=CommandExecutionError('Custom err')) + with patch.dict(linux_acl.__salt__, {'cmd.run': mock}): + with self.assertRaises(CommandExecutionError) as excinfo: + linux_acl.modfacl(*(self.user_acl + self.files), raise_err=True) + self.assertEqual(excinfo.exception.strerror, 'Custom err') def test_delfacl_wo_args(self): for acl in [self.u_acl, self.user_acl, self.g_acl, self.group_acl]: From 5cd71c0036e2e0284e5139527b203ea314f87714 Mon Sep 17 00:00:00 2001 From: Alexandru Bleotu Date: Tue, 25 Jul 2017 21:46:52 +0100 Subject: [PATCH 18/24] Updated raise_err sysdoc in cmdrun.run --- salt/modules/cmdmod.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 920cdffd1a..efb88d563c 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -874,8 +874,9 @@ def run(cmd, :param bool encoded_cmd: Specify if the supplied command is encoded. Only applies to shell 'powershell'. - :pram bool raise_err: Specifies whether to raise a CommandExecutionError. - If False, the error will be logged. Default is False. + :param bool raise_err: Specifies whether to raise a CommandExecutionError. + If False, the error will be logged, but no exception will be raised. + Default is False. .. warning:: This function does not process commands through a shell From c266d507b66c80b690e856a0cc2738acbb503781 Mon Sep 17 00:00:00 2001 From: Nicole Thomas Date: Tue, 25 Jul 2017 16:31:45 -0600 Subject: [PATCH 19/24] Lint: Remove extra whitespace --- salt/master.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/master.py b/salt/master.py index 51acb5962a..d2930377fe 100644 --- a/salt/master.py +++ b/salt/master.py @@ -1305,7 +1305,7 @@ class AESFuncs(object): with salt.utils.files.fopen(cpath, mode) as fp_: if load['loc']: fp_.seek(load['loc']) - + fp_.write(load['data']) return True From 6342971eca0d90de519b04fe13abd69fe61eb989 Mon Sep 17 00:00:00 2001 From: rallytime Date: Thu, 27 Jul 2017 11:43:58 -0600 Subject: [PATCH 20/24] Add a "preflight_cmds" option to salt-cloud This option allows users to specify a list of shell commands that should be executed on the VM right before the bootstrap script runs (or whatever deploy script is set). --- doc/topics/cloud/misc.rst | 21 +++++++++++++++++++++ doc/topics/releases/oxygen.rst | 26 ++++++++++++++++++++++++-- salt/utils/cloud.py | 12 ++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/doc/topics/cloud/misc.rst b/doc/topics/cloud/misc.rst index 3ef1518923..e71f17dddc 100644 --- a/doc/topics/cloud/misc.rst +++ b/doc/topics/cloud/misc.rst @@ -386,3 +386,24 @@ script, a cloud profile using ``file_map`` might look like: file_map: /local/path/to/custom/script: /remote/path/to/use/custom/script /local/path/to/package: /remote/path/to/store/package + +Running Pre-Flight Commands +=========================== + +.. versionadded:: Oxygen + +To execute specified preflight shell commands on a VM before the deploy script is +run, use the ``preflight_cmds`` option. These must be defined as a list in a cloud +configuration file. For example: + +.. code-block:: yaml + + my-cloud-profile: + provider: linode-config + image: Ubuntu 16.04 LTS + size: Linode 2048 + preflight_cmds: + - whoami + - echo 'hello world!' + +These commands will run in sequence **before** the bootstrap script is executed. \ No newline at end of file diff --git a/doc/topics/releases/oxygen.rst b/doc/topics/releases/oxygen.rst index 1fb464d4db..43a4264c67 100644 --- a/doc/topics/releases/oxygen.rst +++ b/doc/topics/releases/oxygen.rst @@ -49,8 +49,30 @@ environments (i.e. ``saltenvs``) have been added: ignore all tags and use branches only, and also to keep SHAs from being made available as saltenvs. -Salt Cloud and Newer PyWinRM Versions -------------------------------------- +Salt Cloud Features +=================== + +Pre-Flight Commands +------------------- + +Support has been added for specified "preflight commands" to run on a VM before +the deploy script is run. These must be defined as a list in a cloud configuration +file. For example: + +.. code-block:: yaml + + my-cloud-profile: + provider: linode-config + image: Ubuntu 16.04 LTS + size: Linode 2048 + preflight_cmds: + - whoami + - echo 'hello world!' + +These commands will run in sequence **before** the bootstrap script is executed. + +Newer PyWinRM Versions +---------------------- Versions of ``pywinrm>=0.2.1`` are finally able to disable validation of self signed certificates. :ref:`Here` for more information. diff --git a/salt/utils/cloud.py b/salt/utils/cloud.py index 654201e1ec..3c8f0ab3b1 100644 --- a/salt/utils/cloud.py +++ b/salt/utils/cloud.py @@ -452,6 +452,9 @@ def bootstrap(vm_, opts): 'maxtries': salt.config.get_cloud_config_value( 'wait_for_passwd_maxtries', vm_, opts, default=15 ), + 'preflight_cmds': salt.config.get_cloud_config_value( + 'preflight_cmds', vm_, __opts__, default=[] + ), } inline_script_kwargs = deploy_kwargs @@ -1465,6 +1468,15 @@ def deploy_script(host, 'Can\'t set ownership for {0}'.format( preseed_minion_keys_tempdir)) + # Run any pre-flight commands before running deploy scripts + preflight_cmds = kwargs.get('preflight_cmds', []) + for command in preflight_cmds: + cmd_ret = root_cmd(command, tty, sudo, **ssh_kwargs) + if cmd_ret: + raise SaltCloudSystemExit( + 'Pre-flight command failed: \'{0}\''.format(command) + ) + # The actual deploy script if script: # got strange escaping issues with sudoer, going onto a From 2cc808e2b8c5ed100c290db6f54f8957814b3e34 Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Thu, 27 Jul 2017 13:56:13 -0600 Subject: [PATCH 21/24] Add different bindpw behavior to the Oxygen release notes. --- doc/topics/releases/oxygen.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/topics/releases/oxygen.rst b/doc/topics/releases/oxygen.rst index 65b182ab2a..da0984b55b 100644 --- a/doc/topics/releases/oxygen.rst +++ b/doc/topics/releases/oxygen.rst @@ -25,6 +25,19 @@ by any master tops matches that are not matched via a top file. To make master tops matches execute first, followed by top file matches, set the new :conf_minion:`master_tops_first` minion config option to ``True``. +LDAP via External Authentication Changes +---------------------------------------- +In this release of Salt, if LDAP Bind Credentials are supplied, then +these credentials will be used for all LDAP access except the first +authentication when a job is submitted. The first authentication will +use the user's credentials as passed on the CLI. This behavior is to +accommodate certain two-factor authentication schemes where the authentication +token can only be used once. + +In previous releases the bind credentials would only be used to determine +the LDAP user's existence and group membership. The user's LDAP credentials +were used from then on. + New GitFS Features ------------------ From 713627863f1c9d41a6458f511fedf3da09b220e9 Mon Sep 17 00:00:00 2001 From: Mike Adams Date: Fri, 28 Jul 2017 20:03:55 -0700 Subject: [PATCH 22/24] Correctly detect Solaris operating systems in Salt 2016.11.x Fixes #39461 --- salt/modules/pkgutil.py | 2 +- salt/modules/solarisips.py | 2 +- salt/modules/solarispkg.py | 2 +- salt/modules/zoneadm.py | 3 +-- salt/modules/zonecfg.py | 3 +-- 5 files changed, 5 insertions(+), 7 deletions(-) diff --git a/salt/modules/pkgutil.py b/salt/modules/pkgutil.py index 9dd294b7e0..818cfc9516 100644 --- a/salt/modules/pkgutil.py +++ b/salt/modules/pkgutil.py @@ -27,7 +27,7 @@ def __virtual__(): ''' Set the virtual pkg module if the os is Solaris ''' - if __grains__['os'] == 'Solaris': + if __grains__['os_family'] == 'Solaris': return __virtualname__ return (False, 'The pkgutil execution module cannot be loaded: ' 'only available on Solaris systems.') diff --git a/salt/modules/solarisips.py b/salt/modules/solarisips.py index b3f48a61cd..79cc510ea3 100644 --- a/salt/modules/solarisips.py +++ b/salt/modules/solarisips.py @@ -56,7 +56,7 @@ def __virtual__(): ''' Set the virtual pkg module if the os is Solaris 11 ''' - if __grains__['os'] == 'Solaris' \ + if __grains__['os_family'] == 'Solaris' \ and float(__grains__['kernelrelease']) > 5.10 \ and salt.utils.which('pkg'): return __virtualname__ diff --git a/salt/modules/solarispkg.py b/salt/modules/solarispkg.py index f51667b6eb..84588976d7 100644 --- a/salt/modules/solarispkg.py +++ b/salt/modules/solarispkg.py @@ -30,7 +30,7 @@ def __virtual__(): ''' Set the virtual pkg module if the os is Solaris ''' - if __grains__['os'] == 'Solaris' and float(__grains__['kernelrelease']) <= 5.10: + if __grains__['os_family'] == 'Solaris' and float(__grains__['kernelrelease']) <= 5.10: return __virtualname__ return (False, 'The solarispkg execution module failed to load: only available ' diff --git a/salt/modules/zoneadm.py b/salt/modules/zoneadm.py index 4edbe15679..d7d21f26dc 100644 --- a/salt/modules/zoneadm.py +++ b/salt/modules/zoneadm.py @@ -61,9 +61,8 @@ def __virtual__(): We are available if we are have zoneadm and are the global zone on Solaris 10, OmniOS, OpenIndiana, OpenSolaris, or Smartos. ''' - ## note: we depend on PR#37472 to distinguish between Solaris and Oracle Solaris if _is_globalzone() and salt.utils.which('zoneadm'): - if __grains__['os'] in ['Solaris', 'OpenSolaris', 'SmartOS', 'OmniOS', 'OpenIndiana']: + if __grains__['os_family'] == 'Solaris': return __virtualname__ return ( diff --git a/salt/modules/zonecfg.py b/salt/modules/zonecfg.py index 2aa9728c06..dfa80920eb 100644 --- a/salt/modules/zonecfg.py +++ b/salt/modules/zonecfg.py @@ -97,9 +97,8 @@ def __virtual__(): We are available if we are have zonecfg and are the global zone on Solaris 10, OmniOS, OpenIndiana, OpenSolaris, or Smartos. ''' - # note: we depend on PR#37472 to distinguish between Solaris and Oracle Solaris if _is_globalzone() and salt.utils.which('zonecfg'): - if __grains__['os'] in ['Solaris', 'OpenSolaris', 'SmartOS', 'OmniOS', 'OpenIndiana']: + if __grains__['os_family'] == 'Solaris': return __virtualname__ return ( From 2e88a35ffc30316585c1dfa3bcf62ce0b7defd16 Mon Sep 17 00:00:00 2001 From: Mike Adams Date: Sat, 29 Jul 2017 08:08:59 -0700 Subject: [PATCH 23/24] Modify __virtual__ to use os grain and look for 'Oracle Solaris' --- salt/modules/zoneadm.py | 2 +- salt/modules/zonecfg.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/modules/zoneadm.py b/salt/modules/zoneadm.py index d7d21f26dc..e8ddda6ee6 100644 --- a/salt/modules/zoneadm.py +++ b/salt/modules/zoneadm.py @@ -62,7 +62,7 @@ def __virtual__(): Solaris 10, OmniOS, OpenIndiana, OpenSolaris, or Smartos. ''' if _is_globalzone() and salt.utils.which('zoneadm'): - if __grains__['os_family'] == 'Solaris': + if __grains__['os'] in ['Oracle Solaris', 'OpenSolaris', 'SmartOS', 'OmniOS', 'OpenIndiana']: return __virtualname__ return ( diff --git a/salt/modules/zonecfg.py b/salt/modules/zonecfg.py index dfa80920eb..7bfd08858c 100644 --- a/salt/modules/zonecfg.py +++ b/salt/modules/zonecfg.py @@ -98,7 +98,7 @@ def __virtual__(): Solaris 10, OmniOS, OpenIndiana, OpenSolaris, or Smartos. ''' if _is_globalzone() and salt.utils.which('zonecfg'): - if __grains__['os_family'] == 'Solaris': + if __grains__['os'] in ['Oracle Solaris', 'OpenSolaris', 'SmartOS', 'OmniOS', 'OpenIndiana']: return __virtualname__ return ( From 6e68abac0e88b30f00393d9c41cfd46bded6816d Mon Sep 17 00:00:00 2001 From: Mike Adams Date: Sat, 29 Jul 2017 08:37:19 -0700 Subject: [PATCH 24/24] Add __virtual__ check for osmajorelease == 10 on Oracle Solaris minions. --- salt/modules/zoneadm.py | 5 +++-- salt/modules/zonecfg.py | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/salt/modules/zoneadm.py b/salt/modules/zoneadm.py index e8ddda6ee6..3ddaa68645 100644 --- a/salt/modules/zoneadm.py +++ b/salt/modules/zoneadm.py @@ -62,9 +62,10 @@ def __virtual__(): Solaris 10, OmniOS, OpenIndiana, OpenSolaris, or Smartos. ''' if _is_globalzone() and salt.utils.which('zoneadm'): - if __grains__['os'] in ['Oracle Solaris', 'OpenSolaris', 'SmartOS', 'OmniOS', 'OpenIndiana']: + if __grains__['os'] in ['OpenSolaris', 'SmartOS', 'OmniOS', 'OpenIndiana']: + return __virtualname__ + elif __grains__['os'] == 'Oracle Solaris' and int(__grains__['osmajorrelease']) == 10: return __virtualname__ - return ( False, '{0} module can only be loaded in a solaris globalzone.'.format( diff --git a/salt/modules/zonecfg.py b/salt/modules/zonecfg.py index 7bfd08858c..cac7bdb7ea 100644 --- a/salt/modules/zonecfg.py +++ b/salt/modules/zonecfg.py @@ -98,9 +98,10 @@ def __virtual__(): Solaris 10, OmniOS, OpenIndiana, OpenSolaris, or Smartos. ''' if _is_globalzone() and salt.utils.which('zonecfg'): - if __grains__['os'] in ['Oracle Solaris', 'OpenSolaris', 'SmartOS', 'OmniOS', 'OpenIndiana']: + if __grains__['os'] in ['OpenSolaris', 'SmartOS', 'OmniOS', 'OpenIndiana']: + return __virtualname__ + elif __grains__['os'] == 'Oracle Solaris' and int(__grains__['osmajorrelease']) == 10: return __virtualname__ - return ( False, '{0} module can only be loaded in a solaris globalzone.'.format(