From 00b13a2e2fab6e9a9635c4df186269a16d9fbfbd Mon Sep 17 00:00:00 2001
From: Balazs Gibizer <balazs.gibizer@ericsson.com>
Date: Mon, 2 Jul 2018 15:21:17 +0200
Subject: [PATCH] Prevent updating an RP's parent to form a loop

Placement had RP loop detection for RP creation but if an RP is created
without a parent (e.g. root RP) then the parent can be set later with a
PUT /resource_providers/{uuid} request by providing the UUID of the
parent. In this code path the loop detection was missing from the
validation. Moreover there are different loop cases for create than for
set. For create the only possible loop is when the RP being created is
points to itself as a parent. However when the parent is provided later
in a PUT the RP being updated can have descendant RPs. Setting a parent
to a descendant also creates a loop.

This patch adds the missing check and returns HTTP 400 if loop is detected.

Closes-Bug: #1779635
Change-Id: I42c91f5f752f0a4fba8b1d95489fc3f87a1c5b6e
---
 .../placement/objects/resource_provider.py      | 17 +++++++++++++++++
 .../placement/gabbits/resource-provider.yaml    | 17 ++++++++++++++---
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/nova/api/openstack/placement/objects/resource_provider.py b/nova/api/openstack/placement/objects/resource_provider.py
index e41bc128a296..85e5d8923a9f 100644
--- a/nova/api/openstack/placement/objects/resource_provider.py
+++ b/nova/api/openstack/placement/objects/resource_provider.py
@@ -1041,6 +1041,23 @@ class ResourceProvider(base.VersionedObject, base.TimestampedObject):
                             action='update',
                             reason=_('re-parenting a provider is not '
                                      'currently allowed.'))
+                if my_ids.parent_uuid is None:
+                    # So the user specifies a parent for an RP that doesn't
+                    # have one. We have to check that by this new parent we
+                    # don't create a loop in the tree. Basically the new parent
+                    # cannot be the RP itself or one of its descendants.
+                    # However as the RP's current parent is None the above
+                    # condition is the same as "the new parent cannot be any RP
+                    # from the current RP tree".
+                    same_tree = ResourceProviderList.get_all_by_filters(
+                        context,
+                        filters={'in_tree': self.uuid})
+                    rp_uuids_in_the_same_tree = [rp.uuid for rp in same_tree]
+                    if parent_uuid in rp_uuids_in_the_same_tree:
+                        raise exception.ObjectActionError(
+                            action='update',
+                            reason=_('creating loop in the provider tree is '
+                                     'not allowed.'))
 
                 updates['root_provider_id'] = parent_ids.root_id
                 updates['parent_provider_id'] = parent_ids.id
diff --git a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml
index c025ddc80dee..98e32268cde4 100644
--- a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml
+++ b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml
@@ -731,6 +731,17 @@ tests:
   data:
       name: parent
       parent_provider_uuid: $ENVIRON['PARENT_PROVIDER_UUID']
-# this reproduces bug 1779635 as this should be HTTP 400 as we have a loop now
-# in the tree
-  status: 200
+  status: 400
+  response_strings:
+      - 'creating loop in the provider tree is not allowed.'
+
+- name: fail updating the parent to point to its child
+  PUT: /resource_providers/$ENVIRON['PARENT_PROVIDER_UUID']
+  request_headers:
+      content-type: application/json
+  data:
+      name: parent
+      parent_provider_uuid: $ENVIRON['RP_UUID']
+  status: 400
+  response_strings:
+      - 'creating loop in the provider tree is not allowed.'