From 6e9901583697e4684478e1dba680738a58cc30c1 Mon Sep 17 00:00:00 2001 From: Tzu-Mainn Chen Date: Tue, 17 Mar 2020 14:37:53 +0000 Subject: [PATCH] Clean up nits from adding additional node update policies This patch resolves some minor nits. Change-Id: I47f301cb4cdc8de158fa52aad35b7136d1356b93 --- ironic/api/controllers/v1/node.py | 2 +- .../unit/api/controllers/v1/test_node.py | 24 ++++++++++++------- ...-info-extra-policies-862b2a70b941cf39.yaml | 4 ++-- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 905795bc8b..7917040095 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -2124,7 +2124,7 @@ class NodesController(rest.RestController): generic_update = True # always do at least one check - if generic_update or policy_checks == []: + if generic_update or not policy_checks: policy_checks.append('baremetal:node:update') rpc_node = api_utils.check_multiple_node_policies_and_retrieve( diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 429fab1b85..7ad2ebe824 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -3400,7 +3400,8 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) - @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve', + autospec=True) def test_patch_policy_update(self, mock_cmnpar): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid()) @@ -3417,7 +3418,8 @@ class TestPatch(test_api_base.BaseApiTest): mock_cmnpar.assert_called_once_with( ['baremetal:node:update'], node.uuid, with_suffix=True) - @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve', + autospec=True) def test_patch_policy_update_none(self, mock_cmnpar): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid()) @@ -3432,7 +3434,8 @@ class TestPatch(test_api_base.BaseApiTest): mock_cmnpar.assert_called_once_with( ['baremetal:node:update'], node.uuid, with_suffix=True) - @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve', + autospec=True) def test_patch_policy_update_extra(self, mock_cmnpar): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid()) @@ -3449,7 +3452,8 @@ class TestPatch(test_api_base.BaseApiTest): mock_cmnpar.assert_called_once_with( ['baremetal:node:update_extra'], node.uuid, with_suffix=True) - @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve', + autospec=True) def test_patch_policy_update_instance_info(self, mock_cmnpar): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid()) @@ -3467,7 +3471,8 @@ class TestPatch(test_api_base.BaseApiTest): ['baremetal:node:update_instance_info'], node.uuid, with_suffix=True) - @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve', + autospec=True) def test_patch_policy_update_generic_and_extra(self, mock_cmnpar): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid()) @@ -3488,7 +3493,8 @@ class TestPatch(test_api_base.BaseApiTest): ['baremetal:node:update_extra', 'baremetal:node:update'], node.uuid, with_suffix=True) - @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve', + autospec=True) def test_patch_policy_update_generic_and_instance_info(self, mock_cmnpar): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid()) @@ -3509,7 +3515,8 @@ class TestPatch(test_api_base.BaseApiTest): ['baremetal:node:update_instance_info', 'baremetal:node:update'], node.uuid, with_suffix=True) - @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve', + autospec=True) def test_patch_policy_update_extra_and_instance_info(self, mock_cmnpar): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid()) @@ -3531,7 +3538,8 @@ class TestPatch(test_api_base.BaseApiTest): 'baremetal:node:update_instance_info'], node.uuid, with_suffix=True) - @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve', + autospec=True) def test_patch_policy_update_generic_extra_instance_info( self, mock_cmnpar): node = obj_utils.create_test_node(self.context, diff --git a/releasenotes/notes/node-update-instance-info-extra-policies-862b2a70b941cf39.yaml b/releasenotes/notes/node-update-instance-info-extra-policies-862b2a70b941cf39.yaml index ce98552398..f7d225e7a6 100644 --- a/releasenotes/notes/node-update-instance-info-extra-policies-862b2a70b941cf39.yaml +++ b/releasenotes/notes/node-update-instance-info-extra-policies-862b2a70b941cf39.yaml @@ -4,5 +4,5 @@ features: Adds ``baremetal:node:update_extra`` and ``baremetal:node:instance_info`` policies to allow finer-grained policy control over node updates. In order to use standalone Ironic to provision a node, a user must be able to - update these attributes, and a lessee should not be able to update all - node attributes. + update ``instance_info`` (and ``extra`` if using metalsmith), and a lessee + should not be able to update all node attributes.