Code review comment for lp:~soren/nova/secgroup-fixes

Revision history for this message
Vish Ishaya (vishvananda) wrote :

This definitely fixes the linked bug, but it doesn't really address the way that security groups with no source ports are "supposed to" work.

According to this bug:

https://bugs.launchpad.net/nova/+bug/829609

When you specify no source group, amazon actually creates 3 separate rules to allow all tcp udp and icmp traffic from the group. So it seems like empty ones actually should be converted.

I think there are two options for this:

a) create the 3 rules when we first receive the request and migrate old empty rules into three rules

b) continue to allow them to be created as is, but have a conversion that displays them as three rules to the client, and implements allow all when modifying ip tables.

If i read your patch correctly, you've done the part of b which makes them allow all traffic, but we still haven't made them display properly to the client. I think I'm ok with this if we add a bug for fixing the client display as well, although I am concerned about certain edge cases. Like what happens if someone tries to revoke one of the faux rules?

review: Needs Information

« Back to merge proposal