Merge "Fixing cluster creation with is_protected field" into stable/liberty

releasenotes/notes/cluster-is-protected-eee9e2bd2595fdc8.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - Cluster with 'is_protected' field can be created now,
+    closed bug #1510929

sahara/api/v10.py

@@ -79,7 +79,7 @@ def clusters_get(cluster_id):
 @rest.patch('/clusters/<cluster_id>')
 @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)
 

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(

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)

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

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