Network ACLs + list_networks

[kylehogan]

functionality to list all networks from #519
depends on #545 as the access parameter for networks was changed

this closes #519, #545, #520, #453, and possibly #496

the functionality for showing connected nodes is outside of show_network() currently, but could be implemented there instead

also affects #566, in a negative way

[kylehogan]

any ideas on why this is failing the Travis checks?
getting a type error that node_register takes only one argument
I am not getting the same error locally and none of the other tests are failing with this line

[henn]

@SahilTikale - does the OBM IPMI change we just merged require a
different config file to load the correct drivers?

[kylehogan]

it was a change to the node_register call that was required by the OBM IPMI change - I didnt have it locally so I didnt run into the issue there and all the other tests were getting fixed by git when it merged.

I just changed my calls to node_register to match the new syntax and it was fine

[henn]

Could you reflect that you are adding access to a network for this and project_remove_network?

[kylehogan]

@xuhang57 Lucas, would you mind reviewing this since the list_networks code is yours? I made a few small changes because of the many to many project:network relationship and that public networks should have project access be 'None', but everything else should be the same.

[xuhang57]

Thanks a lot Kyle! @kylehogan And it looks good to me and I agree with you that we should set them as 'None', which you have already fixed.

[kylehogan]

There is an issue with the ownership of admin created networks now that a network can be accessed by multiple projects.

I believe that the original intent was that if an admin created a network with access 'project1' then project1 would effectively be the 'owner' of the network.
Now that access is a list, all projects in the list have equal access rights to the network, but some functions such as adding a project to the network access list should only be allowed by an admin or the project that 'owns' the network.

If a network was created by a project then the owner is obviously network.creator, but for admin created networks I don't think restricting these functions to admin users is in line with what was originally intended by having an admin create a network to be accessed by a project.

I think it would make sense to have network specific admins to allow there to be a distinction between projects that are allowed to access the network and projects that are allowed to grant/remove access for other projects or delete the network etc.

[kylehogan]

done pending review+discussion

[shuwens]

@henn carefully remind you that your hil is also outdated, don't know whether or not this could help solve the branch conflict yet.

[shuwens]

@henn I dont think the last 2 commit should be here, we have to get rid of it after all.

[zenhack]

I believe network.owner can be None, which denotes admin ownership

[henn]

Good point!

Originally, I was expecting #636 to land first, which would have allowed None. Now that we aren't doing that, I'll have fix this.

[henn]

@shwsun - you're right. I got rid of the 2 travis CI commits.

[henn]

Note that the code for passing GET arguments from the CLI, a13d140, should be code reviewed under #643.

If changes happen there, I'll bring them over to this branch too.

[henn]

Also, for functionality of the optional GET argument in list_network_attachments(), this depends on #644.

[gsilvis]

This is listed as waiting on +2, but it has merge conflicts.

[gsilvis]

Also, it has /sixty-eight/ commits, eight of which are internal merge commits, and quite a few of which are typo fixes that shouldn't have their own commits.

[zenhack]

Indeed, some rebasing/squashing would not go amiss.

[henn]

FYI - since some merging is necessary, the plan discussed between
myself, @pjd-nu and @shwsun is to close this over the next few days and
create a new PR that has this branch merged with the latest master, with
some portion of the commits squashed.

Indeed, some rebasing/squashing would not go amiss.

[zenhack]

@henn, sounds good to me.

[naved001]

Hi @henn is this pull request still active, or will a new pr be created with this branch merged? Thanks!

[zenhack]

Spoke to @henn about the next things for me to work on, first thing on the list is getting this sorted out. I've got a branch on which I'm working on resolving the conflicts and cleaning up the history; I'll open a new pr once it's worth a look. I'm going to close this one.