Ironic

Current API controller design creates routes which shouldn't exist

Bug #1580997 reported by Sam Betts
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ironic
Triaged
Medium
Unassigned

Bug Description

The way that the pecan rest controller works means that any controllers defined as class attributes of the controller become sub-controllers of that controller. This is influences the URL routing created by pecan leading to some unexpected results when we enable using node names in the URLS:

class Nodes(RestController):
    ports = PortsController()

leads to not only the routes:

nodes/<node id>/ports

but also

nodes/ports

nodes/ports is a route that doesn't mean anything to our API and actually causes collisions with node names, resulting in certain node names being disallowed. This can be fixed by writing our controllers using a different method.

Tags: api
Sam Betts (sambetts)
Changed in ironic:
assignee: nobody → Sam Betts (sambetts)
OpenStack Infra (hudson-openstack)
Changed in ironic:
status: New → In Progress
Dmitry Tantsur (divius)
Changed in ironic:
importance: Undecided → High
tags: added: api
OpenStack Infra (hudson-openstack)
Changed in ironic:
assignee: Sam Betts (sambetts) → Vladyslav Drok (vdrok)
Vladyslav Drok (vdrok)
Changed in ironic:
assignee: Vladyslav Drok (vdrok) → Sam Betts (sambetts)
OpenStack Infra (hudson-openstack)
Changed in ironic:
assignee: Sam Betts (sambetts) → Michael Turek (mjturek)
OpenStack Infra (hudson-openstack)
Changed in ironic:
assignee: Michael Turek (mjturek) → Sam Betts (sambetts)
OpenStack Infra (hudson-openstack)
Changed in ironic:
assignee: Sam Betts (sambetts) → Michael Turek (mjturek)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ironic (master)

Reviewed: https://review.openstack.org/314514
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=39e58151afc205fcf10f23256db123bc05007b8b
Submitter: Jenkins
Branch: master

commit 39e58151afc205fcf10f23256db123bc05007b8b
Author: Sam Betts <email address hidden>
Date: Tue May 10 12:29:12 2016 +0100

    Prevent URL collisions with sub-controllers: nodes/ports

    This patch removes the route v1/nodes/ports by removing the ports
    controllers from being a class attribute of the nodes controller, and
    starts using the pecan _lookup function to process sub-resources.

    Co-Authored-By: Vladyslav Drok <email address hidden>
    Partial-Bug: #1580997
    Change-Id: I1f76a5f2ea45629a000e61a215b2b32fdd52fb37

OpenStack Infra (hudson-openstack)
Changed in ironic:
assignee: Michael Turek (mjturek) → Sam Betts (sambetts)
Revision history for this message
Ruby Loo (rloo) wrote :

Another patch to address this: https://review.openstack.org/#/c/315766/

Revision history for this message
Ruby Loo (rloo) wrote :

We discussed this in Monday's ironic meeting [1]. Someone else can summarize (I'm in a hurry.) I'm going to lower the priority for this.

[1] starting at 17:33:53, http://eavesdrop.openstack.org/meetings/ironic/2017/ironic.2017-02-06-17.00.log.html

Changed in ironic:
importance: High → Medium
Revision history for this message
Sam Betts (sambetts) wrote :

During the May 2017 OpenStack summit I spoke to cdent and a few others about whether we should be allowed to remove the unexpected URLs without a microversion bump, once I explained the problem with doing so - routes in pecan are generated using the function definitions which are processed before the microversion being requested can be processed - they seemed to agree that it might be an exception to the general rule but might need more research and might not be worth the time it'll take to fix it.

If we decide not to fix it, then I wonder what we should do with the documentation, should we document the APIs that really exist or the APIs that we want people to use?

We should at least document the node names that you can't use because they collide with the special APIs like states/ports etc.

Revision history for this message
Vladyslav Drok (vdrok) wrote :

This have not had any attention for a while, and half of the patches were not merged. setting it back to triaged for now, it seems like we just need to document this stuff

Changed in ironic:
status: In Progress → Triaged
assignee: Sam Betts (sambetts) → nobody
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on ironic (master)

Change abandoned by Sam Betts (<email address hidden>) on branch: master
Review: https://review.openstack.org/315766
Reason: This is an unmicroversionable change, so abandoning

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Change abandoned by Sam Betts (<email address hidden>) on branch: master
Review: https://review.openstack.org/316149
Reason: Unmicroversionable change abandoning

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.