yahoo-eng-team team mailing list archive

[Chris Dent <cdent+lp@xxxxxxxxxxxxx>]

Public bug reported:

In microversion 1.12 of placement, a schema for allocations was
introduced that required the allocations, project_id and user_id fields.
This schema is used for subsequent microversions, copied and manipulated
as required.

However, it has a flaw. It does not set additionalProperties: False for
the object at which the required fields are set. This means you can add
a field and it glides through. This flaw cascades all the way to the
reshaper (where I had a test that refused to fail in the expected way).

The diff below demonstrates the problem and a potential fix, but this
fix may not be right as it is in the 1.12 microversion and we might want
it on microversion 1.30 and beyond, only (which is a pain).

I think we should just fix it, as below, but I'll let others chime in
too.


diff --git a/nova/api/openstack/placement/schemas/allocation.py b/nova/api/openstack/placement/schemas/allocation.py
index e149ae3beb..796b7c5d01 100644
--- a/nova/api/openstack/placement/schemas/allocation.py
+++ b/nova/api/openstack/placement/schemas/allocation.py
@@ -113,8 +113,9 @@ ALLOCATION_SCHEMA_V1_12 = {
             "type": "string",
             "minLength": 1,
             "maxLength": 255
-        }
+            }
     },
+    "additionalProperties": False,
     "required": [
         "allocations",
         "project_id",
diff --git a/nova/tests/functional/api/openstack/placement/gabbits/allocations-1.28.yaml b/nova/tests/functional/api/openstack/placement/gabbits/allocations-1.28.yaml
index df8fadd66b..04107a996b 100644
--- a/nova/tests/functional/api/openstack/placement/gabbits/allocations-1.28.yaml
+++ b/nova/tests/functional/api/openstack/placement/gabbits/allocations-1.28.yaml
@@ -238,7 +238,9 @@ tests:
       project_id: $ENVIRON['PROJECT_ID']
       user_id: $ENVIRON['USER_ID']
       consumer_generation: null
-  status: 204
+      bad_field: moo
+  #status: 204
+  status: 400
 
 - name: put that allocation to existing consumer
   PUT: /allocations/22222222-2222-2222-2222-222222222222

** Affects: nova
     Importance: Medium
         Status: Confirmed


** Tags: placement

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1782340

Title:
  allocation schema does not set additionalProperties False in all the
  right places

Status in OpenStack Compute (nova):
  Confirmed

Bug description:
  In microversion 1.12 of placement, a schema for allocations was
  introduced that required the allocations, project_id and user_id
  fields. This schema is used for subsequent microversions, copied and
  manipulated as required.

  However, it has a flaw. It does not set additionalProperties: False
  for the object at which the required fields are set. This means you
  can add a field and it glides through. This flaw cascades all the way
  to the reshaper (where I had a test that refused to fail in the
  expected way).

  The diff below demonstrates the problem and a potential fix, but this
  fix may not be right as it is in the 1.12 microversion and we might
  want it on microversion 1.30 and beyond, only (which is a pain).

  I think we should just fix it, as below, but I'll let others chime in
  too.


  diff --git a/nova/api/openstack/placement/schemas/allocation.py b/nova/api/openstack/placement/schemas/allocation.py
  index e149ae3beb..796b7c5d01 100644
  --- a/nova/api/openstack/placement/schemas/allocation.py
  +++ b/nova/api/openstack/placement/schemas/allocation.py
  @@ -113,8 +113,9 @@ ALLOCATION_SCHEMA_V1_12 = {
               "type": "string",
               "minLength": 1,
               "maxLength": 255
  -        }
  +            }
       },
  +    "additionalProperties": False,
       "required": [
           "allocations",
           "project_id",
  diff --git a/nova/tests/functional/api/openstack/placement/gabbits/allocations-1.28.yaml b/nova/tests/functional/api/openstack/placement/gabbits/allocations-1.28.yaml
  index df8fadd66b..04107a996b 100644
  --- a/nova/tests/functional/api/openstack/placement/gabbits/allocations-1.28.yaml
  +++ b/nova/tests/functional/api/openstack/placement/gabbits/allocations-1.28.yaml
  @@ -238,7 +238,9 @@ tests:
         project_id: $ENVIRON['PROJECT_ID']
         user_id: $ENVIRON['USER_ID']
         consumer_generation: null
  -  status: 204
  +      bad_field: moo
  +  #status: 204
  +  status: 400
   
   - name: put that allocation to existing consumer
     PUT: /allocations/22222222-2222-2222-2222-222222222222

To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/1782340/+subscriptions