Current API controller design creates routes which shouldn't exist

Bug #1580997 reported by Sam Betts on 2016-05-12
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 Edit Tag help
Sam Betts (sambetts) on 2016-05-12
Changed in ironic:
assignee: nobody → Sam Betts (sambetts)
Changed in ironic:
status: New → In Progress
Dmitry Tantsur (divius) on 2016-05-16
Changed in ironic:
importance: Undecided → High
tags: added: api
Changed in ironic:
assignee: Sam Betts (sambetts) → Vladyslav Drok (vdrok)
Vladyslav Drok (vdrok) on 2016-07-25
Changed in ironic:
assignee: Vladyslav Drok (vdrok) → Sam Betts (sambetts)
Changed in ironic:
assignee: Sam Betts (sambetts) → Michael Turek (mjturek)
Changed in ironic:
assignee: Michael Turek (mjturek) → Sam Betts (sambetts)
Changed in ironic:
assignee: Sam Betts (sambetts) → Michael Turek (mjturek)

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

Changed in ironic:
assignee: Michael Turek (mjturek) → Sam Betts (sambetts)
Ruby Loo (rloo) wrote :

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

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
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.

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

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

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  Edit
Everyone can see this information.

Other bug subscribers