From cdcf8c93fabed360bf7db07940b2c03a8b557191 Mon Sep 17 00:00:00 2001 From: Andrey Pavlov Date: Wed, 28 Oct 2015 18:12:00 +0300 Subject: [PATCH] Fixing cluster creation with is_protected field Now cluster can be created with is_protected parameter set to True Change-Id: I6c8e10dfc7e3922a1a81e57ff6e762d7a93c389f Closes-bug: #1510929 --- sahara/api/v10.py | 2 +- sahara/db/sqlalchemy/api.py | 4 --- sahara/service/validations/clusters.py | 7 ++++ .../unit/conductor/manager/test_clusters.py | 32 ----------------- .../test_cluster_update_validation.py | 36 +++++++++++++++++++ 5 files changed, 44 insertions(+), 37 deletions(-) diff --git a/sahara/api/v10.py b/sahara/api/v10.py index b78b0bad..fde57b7f 100644 --- a/sahara/api/v10.py +++ b/sahara/api/v10.py @@ -79,7 +79,7 @@ def clusters_get(cluster_id): @rest.patch('/clusters/') @acl.enforce("data-processing:clusters:modify") @v.check_exists(api.get_cluster, 'cluster_id') -@v.validate(v_c_schema.CLUSTER_UPDATE_SCHEMA) +@v.validate(v_c_schema.CLUSTER_UPDATE_SCHEMA, v_c.check_cluster_update) def clusters_update(cluster_id, data): return u.to_wrapped_dict(api.update_cluster, cluster_id, data) diff --git a/sahara/db/sqlalchemy/api.py b/sahara/db/sqlalchemy/api.py index 0644533a..b3241e74 100644 --- a/sahara/db/sqlalchemy/api.py +++ b/sahara/db/sqlalchemy/api.py @@ -244,10 +244,6 @@ def cluster_update(context, cluster_id, values): if cluster is None: raise ex.NotFoundException(cluster_id, _("Cluster id '%s' not found!")) - - validate.check_tenant_for_update(context, cluster) - validate.check_protected_from_update(cluster, values) - cluster.update(values) except db_exc.DBDuplicateEntry as e: raise ex.DBDuplicateEntry( diff --git a/sahara/service/validations/clusters.py b/sahara/service/validations/clusters.py index 24402636..c37ccc2d 100644 --- a/sahara/service/validations/clusters.py +++ b/sahara/service/validations/clusters.py @@ -110,3 +110,10 @@ def check_cluster_delete(cluster_id, **kwargs): acl.check_tenant_for_delete(context.current(), cluster) acl.check_protected_from_delete(cluster) + + +def check_cluster_update(cluster_id, data, **kwargs): + cluster = api.get_cluster(cluster_id) + + acl.check_tenant_for_update(context.current(), cluster) + acl.check_protected_from_update(cluster, data) diff --git a/sahara/tests/unit/conductor/manager/test_clusters.py b/sahara/tests/unit/conductor/manager/test_clusters.py index 4417e8cc..52ce06ae 100644 --- a/sahara/tests/unit/conductor/manager/test_clusters.py +++ b/sahara/tests/unit/conductor/manager/test_clusters.py @@ -368,35 +368,3 @@ class ClusterTest(test_base.ConductorManagerTestCase): self.assertRaises(sa_exc.InvalidRequestError, self.api.cluster_get_all, ctx, **{'badfield': 'somevalue'}) - - def test_cluster_update_when_protected(self): - ctx = context.ctx() - sample = copy.deepcopy(SAMPLE_CLUSTER) - sample['is_protected'] = True - cl = self.api.cluster_create(ctx, sample) - cl_id = cl["id"] - - with testtools.ExpectedException(ex.UpdateFailedException): - try: - self.api.cluster_update(ctx, cl_id, {"name": "cluster"}) - except ex.UpdateFailedException as e: - self.assert_protected_resource_exception(e) - raise e - - self.api.cluster_update(ctx, cl_id, {"name": "cluster", - "is_protected": False}) - - def test_public_cluster_update_from_another_tenant(self): - ctx = context.ctx() - sample = copy.deepcopy(SAMPLE_CLUSTER) - sample['is_public'] = True - cl = self.api.cluster_create(ctx, sample) - cl_id = cl["id"] - ctx.tenant_id = 'tenant_2' - - with testtools.ExpectedException(ex.UpdateFailedException): - try: - self.api.cluster_update(ctx, cl_id, {"name": "cluster"}) - except ex.UpdateFailedException as e: - self.assert_created_in_another_tenant_exception(e) - raise e diff --git a/sahara/tests/unit/service/validation/test_cluster_update_validation.py b/sahara/tests/unit/service/validation/test_cluster_update_validation.py index 263c1a4f..8f04107b 100644 --- a/sahara/tests/unit/service/validation/test_cluster_update_validation.py +++ b/sahara/tests/unit/service/validation/test_cluster_update_validation.py @@ -15,10 +15,14 @@ import mock +import testtools +from sahara import exceptions as ex from sahara.service import api +from sahara.service.validations import clusters as c_val from sahara.service.validations import clusters_schema as c_schema from sahara.tests.unit.service.validation import utils as u +from sahara.tests.unit import testutils as tu class TestClusterUpdateValidation(u.ValidationTestCase): @@ -60,3 +64,35 @@ class TestClusterUpdateValidation(u.ValidationTestCase): "Additional properties are not allowed " "('id' was unexpected)") ) + + @mock.patch('sahara.service.api.get_cluster') + def test_cluster_update_when_protected(self, get_cluster_p): + cluster = tu.create_cluster("cluster1", "tenant_1", "fake", + "0.1", ['ng1'], is_protected=True) + get_cluster_p.return_value = cluster + + # cluster can't be updated if it's marked as protected + with testtools.ExpectedException(ex.UpdateFailedException): + try: + c_val.check_cluster_update(cluster.id, {'name': 'new'}) + except ex.UpdateFailedException as e: + self.assert_protected_resource_exception(e) + raise e + + # cluster can be updated because is_protected flag was set to False + c_val.check_cluster_update( + cluster.id, {'is_protected': False, 'name': 'new'}) + + @mock.patch('sahara.service.api.get_cluster') + def test_public_cluster_update_from_another_tenant(self, get_cluster_p): + cluster = tu.create_cluster("cluster1", "tenant_2", "fake", + "0.1", ['ng1'], is_public=True) + get_cluster_p.return_value = cluster + + # cluster can't be updated from another tenant + with testtools.ExpectedException(ex.UpdateFailedException): + try: + c_val.check_cluster_update(cluster.id, {'name': 'new'}) + except ex.UpdateFailedException as e: + self.assert_created_in_another_tenant_exception(e) + raise e