From 6aee864954069fded4af23d7064d601e71b71f5a Mon Sep 17 00:00:00 2001 From: Ricardo Rocha Date: Tue, 5 Feb 2019 11:42:19 +0100 Subject: [PATCH] Add hidden flag to cluster template Add a new hidden flag to cluster templates. This allows an operator to keep a cluster public (accessible to all users) while not showing them in cluster template listing. Story: 2004941 Task: 29342 Change-Id: Ia2717ca960041753f6e772bf2d41c7f5a196dae6 --- magnum/api/controllers/v1/baymodel.py | 6 +- magnum/api/controllers/v1/cluster_template.py | 17 ++-- magnum/common/exception.py | 3 +- ...2e3c7abc_add_hidden_to_cluster_template.py | 30 +++++++ magnum/db/sqlalchemy/api.py | 17 ++-- magnum/db/sqlalchemy/models.py | 1 + magnum/objects/cluster_template.py | 4 +- .../controllers/v1/test_cluster_template.py | 81 ++++++++++++++++++- magnum/tests/unit/db/test_cluster_template.py | 18 +++++ magnum/tests/unit/db/utils.py | 1 + magnum/tests/unit/objects/test_objects.py | 2 +- 11 files changed, 163 insertions(+), 17 deletions(-) create mode 100644 magnum/db/sqlalchemy/alembic/versions/87e62e3c7abc_add_hidden_to_cluster_template.py diff --git a/magnum/api/controllers/v1/baymodel.py b/magnum/api/controllers/v1/baymodel.py index a506724e31..c9a4b73ad2 100644 --- a/magnum/api/controllers/v1/baymodel.py +++ b/magnum/api/controllers/v1/baymodel.py @@ -138,6 +138,9 @@ class BayModel(base.APIBase): floating_ip_enabled = wsme.wsattr(types.boolean, default=True) """Indicates whether created bays should have a floating ip or not.""" + hidden = wsme.wsattr(types.boolean, default=False) + """Indicates whether the Baymodel is hidden or not.""" + def __init__(self, **kwargs): self.fields = [] for field in objects.ClusterTemplate.fields: @@ -192,6 +195,7 @@ class BayModel(base.APIBase): public=False, master_lb_enabled=False, floating_ip_enabled=True, + hidden=False, ) return cls._convert_with_links(sample, 'http://localhost:9511') @@ -201,7 +205,7 @@ class BayModelPatchType(types.JsonPatchType): _extra_non_removable_attrs = {'/network_driver', '/external_network_id', '/tls_disabled', '/public', '/server_type', '/coe', '/registry_enabled', - '/cluster_distro'} + '/cluster_distro', '/hidden'} class BayModelCollection(collection.Collection): diff --git a/magnum/api/controllers/v1/cluster_template.py b/magnum/api/controllers/v1/cluster_template.py index 37087fda4f..766bd8f017 100644 --- a/magnum/api/controllers/v1/cluster_template.py +++ b/magnum/api/controllers/v1/cluster_template.py @@ -149,6 +149,9 @@ class ClusterTemplate(base.APIBase): user_id = wsme.wsattr(wtypes.text, readonly=True) """User id of the cluster belongs to""" + hidden = wsme.wsattr(types.boolean, default=False) + """Indicates whether the ClusterTemplate is hidden or not.""" + def __init__(self, **kwargs): self.fields = [] for field in objects.ClusterTemplate.fields: @@ -205,7 +208,8 @@ class ClusterTemplate(base.APIBase): updated_at=timeutils.utcnow(), public=False, master_lb_enabled=False, - floating_ip_enabled=True) + floating_ip_enabled=True, + hidden=False) return cls._convert_with_links(sample, 'http://localhost:9511') @@ -214,7 +218,7 @@ class ClusterTemplatePatchType(types.JsonPatchType): _extra_non_removable_attrs = {'/network_driver', '/external_network_id', '/tls_disabled', '/public', '/server_type', '/coe', '/registry_enabled', - '/cluster_distro'} + '/cluster_distro', '/hidden'} class ClusterTemplateCollection(collection.Collection): @@ -390,8 +394,8 @@ class ClusterTemplatesController(base.Controller): cluster_template_dict['cluster_distro'] = image_data['os_distro'] cluster_template_dict['project_id'] = context.project_id cluster_template_dict['user_id'] = context.user_id - # check permissions for making cluster_template public - if cluster_template_dict['public']: + # check permissions for making cluster_template public or hidden + if cluster_template_dict['public'] or cluster_template_dict['hidden']: if not policy.enforce(context, "clustertemplate:publish", None, do_raise=False): raise exception.ClusterTemplatePublishDenied() @@ -440,8 +444,9 @@ class ClusterTemplatesController(base.Controller): new_cluster_template_dict = new_cluster_template.as_dict() attr_validator.validate_os_resources(context, new_cluster_template_dict) - # check permissions when updating ClusterTemplate public flag - if cluster_template.public != new_cluster_template.public: + # check permissions when updating ClusterTemplate public or hidden flag + if (cluster_template.public != new_cluster_template.public or + cluster_template.hidden != new_cluster_template.hidden): if not policy.enforce(context, "clustertemplate:publish", None, do_raise=False): raise exception.ClusterTemplatePublishDenied() diff --git a/magnum/common/exception.py b/magnum/common/exception.py index 4f4b5ab4f9..c2e38b140d 100755 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -238,7 +238,8 @@ class ClusterTemplateReferenced(Invalid): class ClusterTemplatePublishDenied(NotAuthorized): - message = _("Not authorized to set public flag for cluster template.") + message = _("Not authorized to set public or hidden flag for cluster" + " template.") class ClusterNotFound(ResourceNotFound): diff --git a/magnum/db/sqlalchemy/alembic/versions/87e62e3c7abc_add_hidden_to_cluster_template.py b/magnum/db/sqlalchemy/alembic/versions/87e62e3c7abc_add_hidden_to_cluster_template.py new file mode 100644 index 0000000000..c8d0a0f7db --- /dev/null +++ b/magnum/db/sqlalchemy/alembic/versions/87e62e3c7abc_add_hidden_to_cluster_template.py @@ -0,0 +1,30 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +"""add hidden to cluster template + +Revision ID: 87e62e3c7abc +Revises: cbbc65a86986 +Create Date: 2019-02-05 15:35:26.290751 + +""" + +# revision identifiers, used by Alembic. +revision = '87e62e3c7abc' +down_revision = 'cbbc65a86986' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('cluster_template', sa.Column('hidden', sa.Boolean(), + default=False)) diff --git a/magnum/db/sqlalchemy/api.py b/magnum/db/sqlalchemy/api.py index 559c00e255..cf8886be19 100644 --- a/magnum/db/sqlalchemy/api.py +++ b/magnum/db/sqlalchemy/api.py @@ -294,9 +294,15 @@ class Connection(api.Connection): query = model_query(models.ClusterTemplate) query = self._add_tenant_filters(context, query) query = self._add_cluster_template_filters(query, filters) - # include public ClusterTemplates - public_q = model_query(models.ClusterTemplate).filter_by(public=True) + # include public (and not hidden) ClusterTemplates + public_q = model_query(models.ClusterTemplate).filter_by( + public=True, hidden=False) query = query.union(public_q) + # include hidden and public ClusterTemplate if admin + if context.is_admin: + hidden_q = model_query(models.ClusterTemplate).filter_by( + public=True, hidden=True) + query = query.union(hidden_q) return _paginate_query(models.ClusterTemplate, limit, marker, sort_key, sort_dir, query) @@ -362,8 +368,9 @@ class Connection(api.Connection): return query.count() != 0 def _is_publishing_cluster_template(self, values): - if (len(values) == 1 and - 'public' in values and values['public'] is True): + if (len(values) == 1 and ( + ('public' in values and values['public'] is True) or + ('hidden' in values))): return True return False @@ -407,7 +414,7 @@ class Connection(api.Connection): if self._is_cluster_template_referenced(session, ref['uuid']): # NOTE(flwang): We only allow to update ClusterTemplate to be - # public and rename + # public, hidden and rename if (not self._is_publishing_cluster_template(values) and list(six.viewkeys(values)) != ["name"]): raise exception.ClusterTemplateReferenced( diff --git a/magnum/db/sqlalchemy/models.py b/magnum/db/sqlalchemy/models.py index 8a18868b41..382e2a6cef 100644 --- a/magnum/db/sqlalchemy/models.py +++ b/magnum/db/sqlalchemy/models.py @@ -189,6 +189,7 @@ class ClusterTemplate(Base): insecure_registry = Column(String(255, mysql_ndb_type=TINYTEXT)) master_lb_enabled = Column(Boolean, default=False) floating_ip_enabled = Column(Boolean, default=True) + hidden = Column(Boolean, default=False) class X509KeyPair(Base): diff --git a/magnum/objects/cluster_template.py b/magnum/objects/cluster_template.py index 955536d81c..1f32ba9a60 100644 --- a/magnum/objects/cluster_template.py +++ b/magnum/objects/cluster_template.py @@ -42,7 +42,8 @@ class ClusterTemplate(base.MagnumPersistentObject, base.MagnumObject, # Version 1.16: Renamed the class from "BayModel' to 'ClusterTemplate' # Version 1.17: 'coe' field type change to ClusterTypeField # Version 1.18: DockerStorageDriver is a StringField (was an Enum) - VERSION = '1.18' + # Version 1.19: Added 'hidden' field + VERSION = '1.19' dbapi = dbapi.get_instance() @@ -78,6 +79,7 @@ class ClusterTemplate(base.MagnumPersistentObject, base.MagnumObject, 'insecure_registry': fields.StringField(nullable=True), 'master_lb_enabled': fields.BooleanField(default=False), 'floating_ip_enabled': fields.BooleanField(default=True), + 'hidden': fields.BooleanField(default=False), } @staticmethod 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 1e1359a457..d1a882bcc9 100644 --- a/magnum/tests/unit/api/controllers/v1/test_cluster_template.py +++ b/magnum/tests/unit/api/controllers/v1/test_cluster_template.py @@ -41,6 +41,7 @@ class TestClusterTemplateObject(base.TestCase): del cluster_template_dict['server_type'] del cluster_template_dict['master_lb_enabled'] del cluster_template_dict['floating_ip_enabled'] + del cluster_template_dict['hidden'] cluster_template = api_cluster_template.ClusterTemplate( **cluster_template_dict) self.assertEqual(wtypes.Unset, cluster_template.image_id) @@ -50,6 +51,7 @@ class TestClusterTemplateObject(base.TestCase): self.assertEqual('vm', cluster_template.server_type) self.assertFalse(cluster_template.master_lb_enabled) self.assertTrue(cluster_template.floating_ip_enabled) + self.assertFalse(cluster_template.hidden) class TestListClusterTemplate(api_base.FunctionalTest): @@ -62,7 +64,7 @@ class TestListClusterTemplate(api_base.FunctionalTest): 'image_id', 'registry_enabled', 'no_proxy', 'keypair_id', 'https_proxy', 'tls_disabled', 'public', 'labels', 'master_flavor_id', - 'volume_driver', 'insecure_registry') + 'volume_driver', 'insecure_registry', 'hidden') def test_empty(self): response = self.get_json('/clustertemplates') @@ -263,7 +265,8 @@ class TestPatch(api_base.FunctionalTest): public=False, docker_volume_size=20, coe='swarm', - labels={'key1': 'val1', 'key2': 'val2'} + labels={'key1': 'val1', 'key2': 'val2'}, + hidden=False ) def test_update_not_found(self): @@ -347,6 +350,46 @@ class TestPatch(api_base.FunctionalTest): self.cluster_template.uuid) self.assertEqual(response['public'], True) + @mock.patch.object(magnum_policy, 'enforce') + def test_update_hidden_cluster_template_success(self, mock_policy): + mock_policy.return_value = True + response = self.patch_json('/clustertemplates/%s' % + self.cluster_template.uuid, + [{'path': '/hidden', 'value': True, + 'op': 'replace'}]) + self.assertEqual('application/json', response.content_type) + self.assertEqual(200, response.status_code) + + response = self.get_json('/clustertemplates/%s' % + self.cluster_template.uuid) + self.assertTrue(response['hidden']) + + @mock.patch.object(magnum_policy, 'enforce') + def test_update_hidden_cluster_template_fail(self, mock_policy): + mock_policy.return_value = False + self.assertRaises(AppError, self.patch_json, + '/clustertemplates/%s' % self.cluster_template.uuid, + [{'path': '/hidden', 'value': True, + 'op': 'replace'}]) + + @mock.patch.object(magnum_policy, 'enforce') + def test_update_cluster_template_hidden_with_cluster_allow_update( + self, mock_policy): + mock_policy.return_value = True + cluster_template = obj_utils.create_test_cluster_template(self.context) + obj_utils.create_test_cluster( + self.context, cluster_template_id=cluster_template.uuid) + response = self.patch_json('/clustertemplates/%s' % + cluster_template.uuid, + [{'path': '/hidden', + 'value': True, + 'op': 'replace'}], + expect_errors=True) + self.assertEqual(200, response.status_int) + response = self.get_json('/clustertemplates/%s' % + self.cluster_template.uuid) + self.assertEqual(response['hidden'], True) + def test_update_cluster_template_replace_labels_success(self): cluster_template = obj_utils.create_test_cluster_template(self.context) response = self.patch_json('/clustertemplates/%s' % @@ -879,6 +922,40 @@ class TestPost(api_base.FunctionalTest): self.assertNotIn('id', cc_mock.call_args[0][0]) self.assertFalse(cc_mock.call_args[0][0]['public']) + @mock.patch('magnum.api.attr_validator.validate_image') + @mock.patch.object(magnum_policy, 'enforce') + def test_create_cluster_template_hidden_success(self, mock_policy, + mock_image_data): + with mock.patch.object( + self.dbapi, 'create_cluster_template', + wraps=self.dbapi.create_cluster_template) as cc_mock: + mock_policy.return_value = True + mock_image_data.return_value = {'name': 'mock_name', + 'os_distro': 'fedora-atomic'} + bdict = apiutils.cluster_template_post_data(hidden=True) + response = self.post_json('/clustertemplates', bdict) + self.assertTrue(response.json['hidden']) + mock_policy.assert_called_with(mock.ANY, + "clustertemplate:publish", + None, do_raise=False) + cc_mock.assert_called_once_with(mock.ANY) + self.assertNotIn('id', cc_mock.call_args[0][0]) + self.assertTrue(cc_mock.call_args[0][0]['hidden']) + + @mock.patch('magnum.api.attr_validator.validate_image') + @mock.patch.object(magnum_policy, 'enforce') + def test_create_cluster_template_hidden_fail(self, mock_policy, + mock_image_data): + with mock.patch.object(self.dbapi, 'create_cluster_template', + wraps=self.dbapi.create_cluster_template): + # make policy enforcement fail + mock_policy.return_value = False + mock_image_data.return_value = {'name': 'mock_name', + 'os_distro': 'fedora-atomic'} + bdict = apiutils.cluster_template_post_data(hidden=True) + self.assertRaises(AppError, self.post_json, '/clustertemplates', + bdict) + @mock.patch('magnum.api.attr_validator.validate_image') def test_create_cluster_template_with_no_os_distro_image(self, mock_image_data): diff --git a/magnum/tests/unit/db/test_cluster_template.py b/magnum/tests/unit/db/test_cluster_template.py index cb88c5520f..7487ceda16 100644 --- a/magnum/tests/unit/db/test_cluster_template.py +++ b/magnum/tests/unit/db/test_cluster_template.py @@ -93,6 +93,12 @@ class DbClusterTemplateTestCase(base.DbTestCase): self.context, ct['id']) self.assertEqual(ct['uuid'], cluster_template.uuid) + def test_get_cluster_template_by_id_hidden(self): + ct = utils.create_test_cluster_template(user_id='not_me', hidden=True) + cluster_template = self.dbapi.get_cluster_template_by_id( + self.context, ct['id']) + self.assertEqual(ct['uuid'], cluster_template.uuid) + def test_get_cluster_template_by_uuid(self): ct = utils.create_test_cluster_template() cluster_template = self.dbapi.get_cluster_template_by_uuid( @@ -105,6 +111,12 @@ class DbClusterTemplateTestCase(base.DbTestCase): self.context, ct['uuid']) self.assertEqual(ct['id'], cluster_template.id) + def test_get_cluster_template_by_uuid_hidden(self): + ct = utils.create_test_cluster_template(user_id='not_me', hidden=True) + cluster_template = self.dbapi.get_cluster_template_by_uuid( + self.context, ct['uuid']) + self.assertEqual(ct['id'], cluster_template.id) + def test_get_cluster_template_that_does_not_exist(self): self.assertRaises(exception.ClusterTemplateNotFound, self.dbapi.get_cluster_template_by_id, @@ -122,6 +134,12 @@ class DbClusterTemplateTestCase(base.DbTestCase): self.assertEqual(ct['id'], res.id) self.assertEqual(ct['uuid'], res.uuid) + def test_get_cluster_template_by_name_hidden(self): + ct = utils.create_test_cluster_template(user_id='not_me', hidden=True) + res = self.dbapi.get_cluster_template_by_name(self.context, ct['name']) + self.assertEqual(ct['id'], res.id) + self.assertEqual(ct['uuid'], res.uuid) + def test_get_cluster_template_by_name_multiple_cluster_template(self): utils.create_test_cluster_template( id=1, name='ct', diff --git a/magnum/tests/unit/db/utils.py b/magnum/tests/unit/db/utils.py index 5b8ec7ddc3..8db8b3ad16 100644 --- a/magnum/tests/unit/db/utils.py +++ b/magnum/tests/unit/db/utils.py @@ -55,6 +55,7 @@ def get_test_cluster_template(**kw): 'insecure_registry': kw.get('insecure_registry', '10.0.0.1:5000'), 'master_lb_enabled': kw.get('master_lb_enabled', True), 'floating_ip_enabled': kw.get('floating_ip_enabled', True), + 'hidden': kw.get('hidden', False), } diff --git a/magnum/tests/unit/objects/test_objects.py b/magnum/tests/unit/objects/test_objects.py index e6e091430e..b64dfc92de 100644 --- a/magnum/tests/unit/objects/test_objects.py +++ b/magnum/tests/unit/objects/test_objects.py @@ -356,7 +356,7 @@ class TestObject(test_base.TestCase, _TestObject): # https://docs.openstack.org/magnum/latest/contributor/objects.html object_data = { 'Cluster': '1.18-9f0dfcc3e898eef2b9a09647b612adb6', - 'ClusterTemplate': '1.18-7fa94f4fdd027acfb4f022f202afdfb5', + 'ClusterTemplate': '1.19-3b0b2b3933d0955abf3ab40111744960', 'Certificate': '1.1-1924dc077daa844f0f9076332ef96815', 'MyObj': '1.0-34c4b1aadefd177b13f9a2f894cc23cd', 'X509KeyPair': '1.2-d81950af36c59a71365e33ce539d24f9',