github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-09 03:57:41 +00:00
Code Issues Projects Releases Wiki Activity

Fix AWS ip permission comparison on security group

Browse Source 
Change isEqualIPPermission to consider the entire list of security group
ids on when checking if a security group id has already been added.

This is used for example when adding and removing ingress rules to the
cluster nodes from an elastic load balancer. Without this, once there
are multiple load balancers, the method as it stands incorrectly returns
false even if the security group id is in the list of group ids. This
causes a few problems: dangling security groups which fill up an
account's limit since they don't get removed, and inability to recreate
load balancers in certain situations (receiving an
InvalidPermission.Duplicate from AWS when adding the same security
group).
This commit is contained in:
Dogan Narinc and James Ravn 2016-01-26 11:01:15 +00:00 committed by Dogan Narinc and James Ravn
parent 52cb4c1d9d
commit 190c829ac5
2 changed files with 88 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
pkg/cloudprovider/providers/aws/aws.go
View File

@ -1447,23 +1447,32 @@ func isEqualIPPermission(l, r *ec2.IpPermission, compareGroupUserIDs bool) bool
}
}
if len(l.UserIdGroupPairs) != len(r.UserIdGroupPairs) {
for _, leftPair := range l.UserIdGroupPairs {
for _, rightPair := range r.UserIdGroupPairs {
if isEqualUserGroupPair(leftPair, rightPair, compareGroupUserIDs) {
return true
}
}
return false
}
for j := range l.UserIdGroupPairs {
if !isEqualStringPointer(l.UserIdGroupPairs[j].GroupId, r.UserIdGroupPairs[j].GroupId) {
return false
}
if compareGroupUserIDs {
if !isEqualStringPointer(l.UserIdGroupPairs[j].UserId, r.UserIdGroupPairs[j].UserId) {
return false
}
}
}
return true
}
func isEqualUserGroupPair(l, r *ec2.UserIdGroupPair, compareGroupUserIDs bool) bool {
if isEqualStringPointer(l.GroupId, r.GroupId) {
if compareGroupUserIDs {
if isEqualStringPointer(l.UserId, r.UserId) {
return true
}
} else {
return true
}
}
return false
}
// Makes sure the security group includes the specified permissions
// Returns true if and only if changes were made
// The security group must already exist
@ -1478,6 +1487,8 @@ func (s *AWSCloud) ensureSecurityGroupIngress(securityGroupId string, addPermiss
return false, fmt.Errorf("security group not found: %s", securityGroupId)
}
glog.V(2).Infof("Existing security group ingress: %s %v", securityGroupId, group.IpPermissions)
changes := []*ec2.IpPermission{}
for _, addPermission := range addPermissions {
hasUserID := false

67
pkg/cloudprovider/providers/aws/aws_test.go
View File

@ -799,3 +799,70 @@ func TestSubnetIDsinVPC(t *testing.T) {
}
}
func TestIsEqualIPPermissionHandlesMultipleGroupIds(t *testing.T) {
oldIpPermission := ec2.IpPermission {
UserIdGroupPairs: []*ec2.UserIdGroupPair {
&ec2.UserIdGroupPair{GroupId: aws.String("firstGroupId")},
&ec2.UserIdGroupPair{GroupId: aws.String("secondGroupId")},
&ec2.UserIdGroupPair{GroupId: aws.String("thirdGroupId")},
},
}
existingIpPermission := ec2.IpPermission {
UserIdGroupPairs: []*ec2.UserIdGroupPair {
&ec2.UserIdGroupPair{GroupId: aws.String("secondGroupId")},
},
}
newIpPermission := ec2.IpPermission {
UserIdGroupPairs: []*ec2.UserIdGroupPair {
&ec2.UserIdGroupPair{GroupId: aws.String("fourthGroupId")},
},
}
equals := isEqualIPPermission(&existingIpPermission, &oldIpPermission, false)
if !equals {
t.Errorf("Should have been considered equal since first is in the second array of groups")
}
equals = isEqualIPPermission(&newIpPermission, &oldIpPermission, false)
if equals {
t.Errorf("Should have not been considered equal since first is not in the second array of groups")
}
}
func TestIsEqualIPPermissionHandlesMultipleGroupIdsWithUserIds(t *testing.T) {
oldIpPermission := ec2.IpPermission {
UserIdGroupPairs: []*ec2.UserIdGroupPair {
&ec2.UserIdGroupPair{GroupId: aws.String("firstGroupId"), UserId: aws.String("firstUserId")},
&ec2.UserIdGroupPair{GroupId: aws.String("secondGroupId"), UserId: aws.String("secondUserId")},
&ec2.UserIdGroupPair{GroupId: aws.String("thirdGroupId"), UserId: aws.String("thirdUserId")},
},
}
existingIpPermission := ec2.IpPermission {
UserIdGroupPairs: []*ec2.UserIdGroupPair {
&ec2.UserIdGroupPair{GroupId: aws.String("secondGroupId"), UserId: aws.String("secondUserId")},
},
}
newIpPermission := ec2.IpPermission {
UserIdGroupPairs: []*ec2.UserIdGroupPair {
&ec2.UserIdGroupPair{GroupId: aws.String("secondGroupId"), UserId: aws.String("anotherUserId")},
},
}
equals := isEqualIPPermission(&existingIpPermission, &oldIpPermission, true)
if !equals {
t.Errorf("Should have been considered equal since first is in the second array of groups")
}
equals = isEqualIPPermission(&newIpPermission, &oldIpPermission, true)
if equals {
t.Errorf("Should have not been considered equal since first is not in the second array of groups")
}
}