-G (security groups) do not work when running under VPC

This should be upgraded to a Major/Critical bug as those of us running VPC's need to be able to assign security groups to the new machines.

We're scheduling a merge/release of knife-ec2 for next week.

We recently released version 0.5.12 of knife-ec2 which bumps the fog dependency to 1.3. Can someone confirm that moving to this version of fog resolves this problem?

It doesn't solve the problem, but I've found why: if a subnet is defined fog ignores any security group names that are passed. If it's passed a list of security group id's instead, it works. I'm new to both Chef and Ruby, but I'll see if I can fix it in a clean way. I'm attaching the hack I used, with it I can pass a security id to knife ec2's group option, which in turn will pass it to fog.

I've added a security group id option, which is required when using VPC: https://github.com/rgeens/knife-ec2/commits/KNIFE_EC2-45 . Using -G with a VPC is disallowed, since fog ignores it. On a non-VPC instance it's possible to specify both names and ids, they're combined in that case.

I suppose I need to add more unit tests before the fix is acceptable? I also checked if 'knife ec2 server list' worked correctly. VPC security groups are not shown, but this appears to be an issue in fog.

I've added some tests: https://github.com/rgeens/knife-ec2/commits/KNIFE_EC2-45 .

Marking resolved for code review.

Why is it mandatory to require security group ids?

Good question. I double checked and it turns out Fog nicely picks the default VPC security group if the id is not passed. Likewise with the security group name and a non-VPC instance. So my assumption was wrong there and specifying the security group to Fog is not mandatory. I'll remove the restriction.

One of the consequences of removing a mandatory security group id in case of a VPC is that when knife prints the security groups the machine is going to use and no id is specified, I don't yet know the security group id of the default VPC security group that's going to be used. I do know the name, 'default', but if I just print that instead, I'd get a minor inconsistency where it shows the default name if nothing is specified and an id if something is. If I'd want to remove that inconsistency, I'd need to do some sort of lookup through the Amazon API to get the name of the security group the id belongs to. If I'm doing that, I might as well do away with specifying the security group ids in knife altogether, and try to fix the issue on Fog's side. It seems to me that theoretically specifying a subnet id and security group name should be sufficient to uniquely identify the group. I don't know why that's not in Fog.

My current fix is sufficient for my needs, but perhaps leaving knife ec2 as is and attempting to fix this in Fog is better in general. Bryan, what do you think about this?

I independently made effectively the same fix as Raf, mine can be seen here:

https://github.com/jliuhtonen/knife-ec2/commit/04567d2bed9584c539ddabba2c390dea529a5c5e

Raf, from the discussion mentioned in Fog's issue 713 (https://github.com/fog/fog/issues/713) the comment by Estonfer suggests that this is a problem with the AWS API itself so I guess there is no way to fix this in Fog.

I agree that you shouldn't have to define a security group id if you do not want to as it defaults nicely to the 'default' group.

I've removed the mandatory security group id when using a VPC and added some additional logic to ensure knife prints the correct security groups given that: https://github.com/rgeens/knife-ec2/tree/KNIFE_EC2-45 .

Thanks for the link Janne. Maybe it's possible to do an additional Amazon API call in Fog that first looks up the security group id given a subnet id and a security group name, and then pass that result on to the API call they currently use. I haven't looked into that though, since passing the id in knife is good enough for me.

resolving for review.

Looks good. Marking for Merge!

Merged!: dce1d99cb58bb4ff7cc8f674