yahoo-eng-team team mailing list archive

[OpenStack Infra <1782340@xxxxxxxxxxxxxxxxxx>]

Reviewed:  https://review.openstack.org/583907
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=0fc4f95914dec7df68e53514771bf3afe1ee9c4f
Submitter: Zuul
Branch:    master

commit 0fc4f95914dec7df68e53514771bf3afe1ee9c4f
Author: Chris Dent <cdent@xxxxxxxxxxxxx>
Date:   Thu Jul 19 10:40:58 2018 +0100

    [placement] disallow additional fields in allocations
    
    Back in microversion 1.12, when the allocations structure was extended
    to allow project_id and user_id on PUT /allocations/{uuid},
    "additionalProperties" was not set in the JSON schema, so it has been
    possible since then to include unused fields in the input. The schema
    was then reused in the creation of subsequent schema for new
    microversions and for new URIs, such as POST /allocations and the
    forthcoming /reshaper.
    
    This change fixes it by fixing the old microversion. This is the "just
    fix it" option from the discussion on the associated bug. The other
    option is to create a new microversion that corrects the behavior. This
    is more complex than it might initially sound because of the way in
    which the original schema is used to compose new ones.
    
    Change-Id: Ied464744803864e61a45e03c559760a8a2e2581f
    Closes-Bug: #1782340


** Changed in: nova
       Status: In Progress => Fix Released

-- 
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):
  Fix Released

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