Fix role validation in release PUT handler
Plugins' roles can be assigned to some nodes, so we can't assume that release roles is be a superset of node roles everywhere. So check that there are no nodes in all clusters, which have roles assigned, that we are trying top delete. Change-Id: Ia10f7b0662866da0872863bba7f97c8ad361fcf4 Closes-Bug: #1625592
This commit is contained in:
parent
c173e7c693
commit
78fbccc9ac
|
@ -94,27 +94,29 @@ class ReleaseValidator(BasicValidator):
|
||||||
)
|
)
|
||||||
|
|
||||||
if 'roles_metadata' in d:
|
if 'roles_metadata' in d:
|
||||||
new_roles = set(d['roles_metadata'])
|
deleted_roles = (set(instance.roles_metadata) -
|
||||||
clusters = [cluster.id for cluster in instance.clusters]
|
set(d['roles_metadata']))
|
||||||
|
clusters_ids = (cluster.id for cluster in instance.clusters)
|
||||||
|
|
||||||
new_roles_array = sa.cast(
|
deleted_roles_array = sa.cast(
|
||||||
psql.array(new_roles),
|
psql.array(deleted_roles),
|
||||||
psql.ARRAY(sa.String(consts.ROLE_NAME_MAX_SIZE)))
|
psql.ARRAY(sa.String(consts.ROLE_NAME_MAX_SIZE)))
|
||||||
|
|
||||||
node = db().query(models.Node).filter(
|
node = db().query(models.Node).filter(
|
||||||
models.Node.cluster_id.in_(clusters)
|
models.Node.cluster_id.in_(clusters_ids)
|
||||||
).filter(sa.not_(sa.and_(
|
).filter(sa.or_(
|
||||||
models.Node.roles.contained_by(new_roles_array),
|
models.Node.roles.overlap(deleted_roles_array),
|
||||||
models.Node.pending_roles.contained_by(new_roles_array)
|
models.Node.pending_roles.overlap(deleted_roles_array)
|
||||||
))).first()
|
)).first()
|
||||||
|
|
||||||
if node:
|
if node:
|
||||||
used_role = set(node.roles + node.pending_roles)
|
used_role = set(node.roles + node.pending_roles)
|
||||||
used_role -= new_roles
|
used_role = used_role.intersection(deleted_roles)
|
||||||
|
|
||||||
raise errors.CannotDelete(
|
raise errors.CannotDelete(
|
||||||
"Cannot delete roles already assigned "
|
"The following roles: {0} cannot be deleted "
|
||||||
"to nodes: {0}".format(','.join(used_role))
|
"since they are already assigned "
|
||||||
|
"to nodes.".format(','.join(used_role))
|
||||||
)
|
)
|
||||||
|
|
||||||
return d
|
return d
|
||||||
|
|
|
@ -146,5 +146,6 @@ class TestRoles(BaseIntegrationTest):
|
||||||
self.assertEqual(resp.status_code, 400)
|
self.assertEqual(resp.status_code, 400)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
resp.json_body["message"],
|
resp.json_body["message"],
|
||||||
"Cannot delete roles already assigned to nodes: controller"
|
"The following roles: controller cannot be deleted "
|
||||||
|
"since they are already assigned to nodes."
|
||||||
)
|
)
|
||||||
|
|
|
@ -16,6 +16,7 @@ from oslo_serialization import jsonutils
|
||||||
from nailgun.api.v1.validators.release import ReleaseValidator
|
from nailgun.api.v1.validators.release import ReleaseValidator
|
||||||
from nailgun import errors
|
from nailgun import errors
|
||||||
from nailgun.test.base import BaseTestCase
|
from nailgun.test.base import BaseTestCase
|
||||||
|
from nailgun.utils import reverse
|
||||||
|
|
||||||
|
|
||||||
class TestReleaseValidator(BaseTestCase):
|
class TestReleaseValidator(BaseTestCase):
|
||||||
|
@ -62,3 +63,40 @@ class TestReleaseValidator(BaseTestCase):
|
||||||
|
|
||||||
def test_default_are_good(self):
|
def test_default_are_good(self):
|
||||||
self.validator.validate(self.get_release(self.release))
|
self.validator.validate(self.get_release(self.release))
|
||||||
|
|
||||||
|
def test_release_delete_role(self):
|
||||||
|
cluster = self.env.create(
|
||||||
|
nodes_kwargs=[{
|
||||||
|
"roles": ['test_plugin_role_1'],
|
||||||
|
"pending_roles": ['cinder', 'test_plugin_role_2'],
|
||||||
|
}])
|
||||||
|
|
||||||
|
resp = self.app.get(
|
||||||
|
reverse('ReleaseHandler',
|
||||||
|
kwargs={'obj_id': cluster.release.id}),
|
||||||
|
headers=self.default_headers
|
||||||
|
)
|
||||||
|
|
||||||
|
data = dict(resp.json_body)
|
||||||
|
resp = self.app.put(
|
||||||
|
reverse('ReleaseHandler',
|
||||||
|
kwargs={'obj_id': cluster.release.id}),
|
||||||
|
params=jsonutils.dumps(data),
|
||||||
|
headers=self.default_headers,
|
||||||
|
expect_errors=False
|
||||||
|
)
|
||||||
|
|
||||||
|
data['roles_metadata'].pop('cinder')
|
||||||
|
resp = self.app.put(
|
||||||
|
reverse('ReleaseHandler',
|
||||||
|
kwargs={'obj_id': cluster.release.id}),
|
||||||
|
params=jsonutils.dumps(data),
|
||||||
|
headers=self.default_headers,
|
||||||
|
expect_errors=True
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assertEqual(resp.status_code, 400)
|
||||||
|
self.assertIn(
|
||||||
|
"The following roles: cinder cannot be deleted",
|
||||||
|
resp.json_body["message"],
|
||||||
|
)
|
||||||
|
|
Loading…
Reference in New Issue