From 2fcaf363a965a4575fd3bc1c7d83e453f3ff27f0 Mon Sep 17 00:00:00 2001 From: Feilong Wang Date: Fri, 29 Mar 2019 15:56:29 +1300 Subject: [PATCH] Allow admin update cluster/template in any project Now Magnum supports list, get, delete user's cluster/template by admin, but not allowed for updating. We're seeing this could be a very useful feature for us, since sometimes we need to help our customer to update their templates or clusters on behalf. Task: 30251 Story: 2005323 Change-Id: I3ab1d4583b5eb3d1c377e46fd73347c2477c3e08 --- magnum/api/controllers/v1/cluster.py | 7 ++++++- magnum/api/controllers/v1/cluster_template.py | 8 ++++++-- magnum/common/policies/cluster.py | 11 +++++++++++ magnum/common/policies/cluster_template.py | 11 +++++++++++ .../tests/unit/api/controllers/v1/test_cluster.py | 13 +++++++++++++ .../api/controllers/v1/test_cluster_template.py | 14 ++++++++++++++ 6 files changed, 61 insertions(+), 3 deletions(-) diff --git a/magnum/api/controllers/v1/cluster.py b/magnum/api/controllers/v1/cluster.py index 971ac2ea70..0d7f07efa5 100755 --- a/magnum/api/controllers/v1/cluster.py +++ b/magnum/api/controllers/v1/cluster.py @@ -408,7 +408,7 @@ class ClustersController(base.Controller): # parameter, currently the design is allowing admin user to list # all clusters from all projects. But the all_tenants is one of # the condition to do project filter in DB API. And it's also used - # by periodic tasks. So the could be removed in the future and + # by periodic tasks. So this could be removed in the future and # a new parameter 'project_id' would be added so that admin user # can list clusters for a particular project. context.all_tenants = True @@ -541,6 +541,11 @@ class ClustersController(base.Controller): def _patch(self, cluster_ident, patch): context = pecan.request.context + if context.is_admin: + policy.enforce(context, "cluster:update_all_projects", + action="cluster:update_all_projects") + context.all_tenants = True + cluster = api_utils.get_resource('Cluster', cluster_ident) policy.enforce(context, 'cluster:update', cluster.as_dict(), action='cluster:update') diff --git a/magnum/api/controllers/v1/cluster_template.py b/magnum/api/controllers/v1/cluster_template.py index 766bd8f017..8fe7d50986 100644 --- a/magnum/api/controllers/v1/cluster_template.py +++ b/magnum/api/controllers/v1/cluster_template.py @@ -275,7 +275,7 @@ class ClusterTemplatesController(base.Controller): # parameter, currently the design is allowing admin user to list # all clusters from all projects. But the all_tenants is one of # the condition to do project filter in DB API. And it's also used - # by periodic tasks. So the could be removed in the future and + # by periodic tasks. So this could be removed in the future and # a new parameter 'project_id' would be added so that admin user # can list clusters for a particular project. context.all_tenants = True @@ -356,7 +356,7 @@ class ClusterTemplatesController(base.Controller): # parameter, currently the design is allowing admin user to list # all clusters from all projects. But the all_tenants is one of # the condition to do project filter in DB API. And it's also used - # by periodic tasks. So the could be removed in the future and + # by periodic tasks. So this could be removed in the future and # a new parameter 'project_id' would be added so that admin user # can list clusters for a particular project. context.all_tenants = True @@ -428,6 +428,10 @@ class ClusterTemplatesController(base.Controller): ClusterTemplate. """ context = pecan.request.context + if context.is_admin: + policy.enforce(context, 'clustertemplate:update_all_projects', + action='clustertemplate:update_all_projects') + context.all_tenants = True cluster_template = api_utils.get_resource('ClusterTemplate', cluster_template_ident) policy.enforce(context, 'clustertemplate:update', diff --git a/magnum/common/policies/cluster.py b/magnum/common/policies/cluster.py index 77e6b79ab6..abace3c6f4 100644 --- a/magnum/common/policies/cluster.py +++ b/magnum/common/policies/cluster.py @@ -129,6 +129,17 @@ rules = [ } ] ), + policy.DocumentedRuleDefault( + name=CLUSTER % 'update_all_projects', + check_str=base.RULE_ADMIN_API, + description='Update an existing cluster.', + operations=[ + { + 'path': '/v1/clusters/{cluster_ident}', + 'method': 'PATCH' + } + ] + ), policy.DocumentedRuleDefault( name=CLUSTER % 'resize', check_str=base.RULE_DENY_CLUSTER_USER, diff --git a/magnum/common/policies/cluster_template.py b/magnum/common/policies/cluster_template.py index ac9844b269..bf149752fa 100644 --- a/magnum/common/policies/cluster_template.py +++ b/magnum/common/policies/cluster_template.py @@ -130,6 +130,17 @@ rules = [ } ] ), + policy.DocumentedRuleDefault( + name=CLUSTER_TEMPLATE % 'update_all_projects', + check_str=base.RULE_ADMIN_API, + description='Update an existing cluster template.', + operations=[ + { + 'path': '/v1/clustertemplate/{clustertemplate_ident}', + 'method': 'PATCH' + } + ] + ), policy.DocumentedRuleDefault( name=CLUSTER_TEMPLATE % 'publish', check_str=base.RULE_ADMIN_API, diff --git a/magnum/tests/unit/api/controllers/v1/test_cluster.py b/magnum/tests/unit/api/controllers/v1/test_cluster.py index 94c92e6fa5..3da80e776b 100644 --- a/magnum/tests/unit/api/controllers/v1/test_cluster.py +++ b/magnum/tests/unit/api/controllers/v1/test_cluster.py @@ -502,6 +502,19 @@ class TestPatch(api_base.FunctionalTest): self.assertEqual(400, response.status_code) self.assertTrue(response.json['errors']) + @mock.patch("magnum.common.policy.enforce") + @mock.patch("magnum.common.context.make_context") + def test_update_cluster_as_admin(self, mock_context, mock_policy): + temp_uuid = uuidutils.generate_uuid() + obj_utils.create_test_cluster(self.context, uuid=temp_uuid) + self.context.is_admin = True + response = self.patch_json('/clusters/%s' % temp_uuid, + [{'path': '/node_count', + 'value': 4, + 'op': 'replace'}]) + self.assertEqual('application/json', response.content_type) + self.assertEqual(202, response.status_code) + class TestPost(api_base.FunctionalTest): def setUp(self): diff --git a/magnum/tests/unit/api/controllers/v1/test_cluster_template.py b/magnum/tests/unit/api/controllers/v1/test_cluster_template.py index d1a882bcc9..dc19b97aa3 100644 --- a/magnum/tests/unit/api/controllers/v1/test_cluster_template.py +++ b/magnum/tests/unit/api/controllers/v1/test_cluster_template.py @@ -597,6 +597,20 @@ class TestPatch(api_base.FunctionalTest): self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['errors']) + @mock.patch("magnum.common.policy.enforce") + @mock.patch("magnum.common.context.make_context") + def test_update_cluster_template_as_admin(self, mock_context, mock_policy): + temp_uuid = uuidutils.generate_uuid() + obj_utils.create_test_cluster_template(self.context, uuid=temp_uuid, + project_id=temp_uuid) + self.context.is_admin = True + response = self.patch_json('/clustertemplates/%s' % temp_uuid, + [{'path': '/name', + 'value': 'cluster_model_example_B', + 'op': 'replace'}], + expect_errors=True) + self.assertEqual(200, response.status_int) + class TestPost(api_base.FunctionalTest):