Refactor argument parsing, validation, error reporting in REST API

[Jim Baker]

Various fixes for Craton API we need to do:

* Better input validation, such as checking bad IP addresses before
  insertion corrupts subsequent database usage
* Reporting of errors, especially CRUD errors such as duplicate
  resources that violate uniqueness indexes
* Usage consistency - /v1/netdevices?region_id=2 vs /v1/hosts?region=2
* Support for queries like so /v1/hosts?region=1&vars=k1:v1,k2=v2 -
  this requires relaxing the current input validation

(We may want to split into separate bugs, but it is possible this can be reasonably done in a single refactoring change that's not so big - they are all related.)

Let's look at input validation. For example we will attempt to create
a new NetDevice with the ip_address = 'bad-ip':

curl http://127.0.0.1:8080/v1/netdevices -H "Content-Type: application/json" -d '{"region_id": 2, "name": "cabinet-1", "project_id": 1, "hostname": "cabinet_1.example.com", "device_type": "switch", "ip_address": "bad-ip", "vlans": "[1,2,3]"}' -H "Content-Type: application/json" -H "X-Auth-Token: demo" -H "X-Auth-User: demo" -H "X-Auth-Project: 1"

The inventory entry is successfully created, as reported:

{"parent_id": null, "name": "cabinet_1.example.com", "id": 19, "cell_id": null, "created_at": "2016-09-13T14:17:27.215343", "updated_at": null, "type": "net_devices", "model_name": null, "region_id": 2, "note": null, "ip_address": "bad-ip", "variable_association_id": 26, "vlans": "[1,2,3]", "variable_association": {"created_at": "2016-09-13T14:17:27.213470", "discriminator": "device", "updated_at": null, "parent": {"parent_id": null, "name": "cabinet_1.example.com", "id": 19, "cell_id": null, "created_at": "2016-09-13T14:17:27.215343", "updated_at": null, "type": "net_devices", "model_name": null, "region_id": 2, "note": null, "ip_address": "bad-ip", "variable_association_id": 26, "vlans": "[1,2,3]", "variable_association": null, "access_secret_id": null, "os_version": null, "project_id": 1, "active": true, "device_type": "switch"}, "id": 26}, "access_secret_id": null, "os_version": null, "project_id": 1, "active": true, "device_type": "switch"}

Subsequent queries of any network devices

curl http://127.0.0.1:8080/v1/netdevices?region_id=2 -H "X-Auth-Token: demo" -H "X-Auth-User: demo" -H "X-Auth-Project: 1"

will then result in messages like so:

{
  "message": "Unknown Error",
  "status": 500
}

The underlying error is reported in the API server logs:

172.17.0.1 - - [13/Sep/2016 14:19:44] "GET /v1/netdevices?region_id=2 HTTP/1.1" 500 51
2016-09-13 14:20:21.458 188 INFO craton.api.v1.resources.inventory.networks [req-e40f8ae3-e9c8-48cc-9730-24a25e286613 demo 1 - - -] Getting all network devices that match filters {}
2016-09-13 14:20:21.466 188 ERROR craton.api.v1.resources.inventory.networks [req-e40f8ae3-e9c8-48cc-9730-24a25e286613 demo 1 - - -] Error during net devices get: 'bad-ip' does not appear to be an IPv4 or IPv6 address

Note that any filtering query on netdevices will fail with the same
error. Likely this is the correct error here - we should not
necessarily report details on database corruption.

After manually cleaning up the database, we can try another
interaction where we insert a duplicate. Run the following command
twice:

curl http://127.0.0.1:8080/v1/netdevices -H "Content-Type: application/json" -d '{"region_id": 2, "name": "cabinet-1", "project_id": 1, "hostname": "cabinet_1.example.com", "device_type": "switch", "ip_address": "10.1.1.1", "vlans": "[1,2,3]"}' -H "Content-Type: application/json" -H "X-Auth-Token: demo" -H "X-Auth-User: demo" -H "X-Auth-Project: 1"

The second attempt results in

{
  "message": "Unknown Error",
  "status": 500
}

with the log

172.17.0.1 - - [13/Sep/2016 15:21:19] "POST /v1/netdevices HTTP/1.1" 200 964
2016-09-13 15:21:20.543 188 ERROR craton.api.v1.resources.inventory.networks [req-d9b2f41d-32e4-4a04-afbf-cc1de4cd9d97 demo 1 - - -] Error during net device create: (_mysql_exceptions.IntegrityError) (1062, "Duplicate entry '2-cabinet_1.example.com' for key 'uq_device0regionid0name'") [SQL: 'INSERT INTO devices (created_at, updated_at, type, name, region_id, cell_id, project_id, access_secret_id, parent_id, ip_address, device_type, active, note, variable_association_id) VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s)'] [parameters: (datetime.datetime(2016, 9, 13, 15, 21, 20, 540552), None, 'net_devices', 'cabinet_1.example.com', 2, None, 1, None, None, '10.1.1.1', 'switch', 1, None, 31)]

It should be instead something like 409: Duplicate entry, with the API server supporting bubbling up integrity errors like this as appropriate user friendly exceptions.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/378977
Committed: https://git.openstack.org/cgit/openstack/craton/commit/?id=33349891a7e28ff9d9f58a5e5e654e653ed293a9
Submitter: Jenkins
Branch: master

commit 33349891a7e28ff9d9f58a5e5e654e653ed293a9
Author: Jim Baker <email address hidden>
Date: Tue Oct 4 08:45:17 2016 -0600

    Refactor REST API support to remove boilerplate

    This change removes the following boilerplate:

    * Most filter setup, along with output to a JSON response, is now
      captured in the new craton.api.v1.base.filtered_context decorator.

    * Provides standard exception to HTTP status code mapping using the
      new craton.api.v1.base.http_codes decorator.

    There are additional opportunities for boilerplate removal, especially
    in scenarios where we have multiple ways of looking up a specific cell
    or region; but this boilerplate is not addressed in this fix.

    This change does not implement related opportunities for cleanup, such
    as import reordering or doc strings. These should be in separate
    changes, per discussion on Gerrit.

    In addition, this change now returns 204 upon DELETE in the API; and
    adds version info to requirements.txt for Flask to conform to Jenkins
    gate check.

    Change-Id: I7da8e9620c2fb07ebd3953cc6d76d47c08651fe7
    Partial-Bug: 1623095
    Closes-Bug: 1628050

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/420142

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/420143

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/420142
Committed: https://git.openstack.org/cgit/openstack/craton/commit/?id=3fd67e50029d1decc13e4e9a775fde7d95304a7a
Submitter: Jenkins
Branch: master

commit 3fd67e50029d1decc13e4e9a775fde7d95304a7a
Author: git-harry <email address hidden>
Date: Fri Jan 13 17:34:21 2017 +0000

    Add context to function signature of all requests

    This commit standardises the way that the context is accessed by methods
    in the API classes. This change takes the approach in filtered_context
    and applies it across all request methods so that they receive it as an
    argument. This approach should simplify the functions, reduce the
    duplication of code defining where the context is stored as well as
    remove the need to access a global variable.

    Change-Id: Id5dfe7b3ab5f71af915634a5b9b58272c120701d
    Related-Bug: 1623095

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/420143
Committed: https://git.openstack.org/cgit/openstack/craton/commit/?id=a894913ae6550cafa1b5208c4a88343c1768593e
Submitter: Jenkins
Branch: master

commit a894913ae6550cafa1b5208c4a88343c1768593e
Author: git-harry <email address hidden>
Date: Fri Jan 13 18:48:44 2017 +0000

    Pass request data as an argument to API methods

    This commit generalises the approach taken by filtered_context; the
    methods no longer access the request data directly but instead receive
    it as an argument.

    This change helps to standardise the code base. Also, by automatically
    supplying the data as an argument, it reduces the likelihood that the
    validation process will be bypassed when any new functionality is added.
    In earlier version, some parts of the API were directly accessing the
    raw request data via 'request.json' instead of the validated data that
    was in 'g.json'.

    Change-Id: Icba9612ef5fa968de706ede610cce111ec9723c1
    Related-Bug: 1623095