revert- remove router dict's gw_port information

Bug #1079206 reported by yong sheng gong
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Fix Released
High
yong sheng gong

Bug Description

gw_port should not be in router's dict since it maybe involve security problem and break the policy engine that is guard the API.

we will have special cli command for us to show these ports for a router.

Revision history for this message
yong sheng gong (gongysh) wrote :
Download full text (4.5 KiB)

On 11/02/2012 06:40 AM, Salvatore Orlando wrote:
> HI,
> I am becoming aware of this patch just now, and I would like to thank Dan for raising the issue.
> Bug 1069782 was initially filed for adding convenience client-side commands for retrieving router interfaces, and I believe it was not supposed to have any server side change.
> In general we should avoid being 'trigger-happy' with changes on the tenant-facing API, even when the change appears totally reasonable and improves the API. Keeping that in mind, this particular change won't break backward compatibility, so it's not super-bad. However, it will break the default policy engine rules - or, to be more precise, it will circumvent the policy engine. Indeed it won't matter what is the policy for ports on external network, you'll end up showing some details about it. You might argue that this is something that needs to be addressed in the policy engine, and I would agree with you.
> Dan proposes two solution which I believe are dictated by a "safety first" approach. They both seem reasonable to me. The client-side option looks cleaner but less efficient, where the admin-only option would be more efficient, and still safe (although I might have some reservation if the admin-only constraint becomes hard coded).
> Salvatore
> On 1 November 2012 22:11, Dan Wendlandt <email address hidden> wrote:
>
> Hi team,
> I have two concerns with the commit below, one general, one specific.
> First off, the general concern: this change changes our tenant-facing API, but it seems that there was no real public discussion or notification of this change. Our tenant-facing APIs, including extensions, must be stable, consistent, and documented. I would argue that changes to the API should be part of blueprints, which will trigger a discussion about API consistency, documentation, etc.
> Second, the specific one: I want to explain the reasoning why this field was not included in the original version of the L3 API, and why my current thinking is that it probably should not be added. The model we were going with for L3 routers is that the details of the gateway are abstracted from the tenant. In particular, the port used to "hold" the IP allocation for gateway and floating-ip ports are not visible to a common tenant and should not be manipulated directly. They are only visible to an admin user. As a result, I would argue that include the gw port info in the JSON that is visible to all tenants doesn't really make sense. We could consider adding it as an admin only attribute (similar to how the provider network attributes are included in a network, but only for admins), though this info is already available to admin users if they search for ports that have the device_owner=<router-id> and device_owner=network:router_gateway . We could even has the CLI do this as part of the router-show command, if the user was an admin.
> I think we can have a technical discussions about the second issue, but my most major concern is actually the first issue in that we need have a more deliberate process when it comes to making changes to tenant-facing APIs.
> Dan
>
> p.s. if we end up deciding to...

Read more...

Changed in quantum:
milestone: none → grizzly-1
Gary Kotton (garyk)
Changed in quantum:
status: New → Confirmed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to quantum (master)

Fix proposed to branch: master
Review: https://review.openstack.org/16194

Changed in quantum:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to quantum (master)

Reviewed: https://review.openstack.org/16194
Committed: http://github.com/openstack/quantum/commit/b4a0f659f678d80d562de9f57b2112a018fd4d45
Submitter: Jenkins
Branch: master

commit b4a0f659f678d80d562de9f57b2112a018fd4d45
Author: gongysh <email address hidden>
Date: Thu Nov 15 23:50:22 2012 +0800

    Revert "Put gw_port into router dict result."

    This reverts commit c1c19b11792e8e220b11466051739ea5b4279a7a.

    Bug #1079206

    Change-Id: Ia5fd84470779846749c00bb7dae7198055388347

Changed in quantum:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in quantum:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in quantum:
milestone: grizzly-1 → 2013.1
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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