github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-13 22:05:59 +00:00
Code Issues Projects Releases Wiki Activity

Fix removal of aws security group ingress

Browse Source 
The ip permission method now checks for containment, not equality, so
order of parameters matter. This change fixes
`removeSecurityGroupIngress` to pass in the removal permission first to
compare against the existing permission.
This commit is contained in:
James Ravn 2016-01-26 13:57:52 +00:00
parent 190c829ac5
commit b4bbc3b3ef
2 changed files with 20 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1418,6 +1418,7 @@ func isEqualIntPointer(l, r *int64) bool {
} }
return *l == *r return *l == *r
} }
func isEqualStringPointer(l, r *string) bool { func isEqualStringPointer(l, r *string) bool {
if l == nil { if l == nil {
return r == nil return r == nil
@ -1428,27 +1429,27 @@ func isEqualStringPointer(l, r *string) bool {
return *l == *r return *l == *r
} }
func isEqualIPPermission(l, r *ec2.IpPermission, compareGroupUserIDs bool) bool { func ipPermissionExists(newPermission, existing *ec2.IpPermission, compareGroupUserIDs bool) bool {
if !isEqualIntPointer(l.FromPort, r.FromPort) { if !isEqualIntPointer(newPermission.FromPort, existing.FromPort) {
return false return false
} }
if !isEqualIntPointer(l.ToPort, r.ToPort) { if !isEqualIntPointer(newPermission.ToPort, existing.ToPort) {
return false return false
} }
if !isEqualStringPointer(l.IpProtocol, r.IpProtocol) { if !isEqualStringPointer(newPermission.IpProtocol, existing.IpProtocol) {
return false return false
} }
if len(l.IpRanges) != len(r.IpRanges) { if len(newPermission.IpRanges) != len(existing.IpRanges) {
return false return false
} }
for j := range l.IpRanges { for j := range newPermission.IpRanges {
if !isEqualStringPointer(l.IpRanges[j].CidrIp, r.IpRanges[j].CidrIp) { if !isEqualStringPointer(newPermission.IpRanges[j].CidrIp, existing.IpRanges[j].CidrIp) {
return false return false
} }
} }
for _, leftPair := range l.UserIdGroupPairs { for _, leftPair := range newPermission.UserIdGroupPairs {
for _, rightPair := range r.UserIdGroupPairs { for _, rightPair := range existing.UserIdGroupPairs {
if isEqualUserGroupPair(leftPair, rightPair, compareGroupUserIDs) { if isEqualUserGroupPair(leftPair, rightPair, compareGroupUserIDs) {
return true return true
} }
@ -1460,6 +1461,7 @@ func isEqualIPPermission(l, r *ec2.IpPermission, compareGroupUserIDs bool) bool
} }
func isEqualUserGroupPair(l, r *ec2.UserIdGroupPair, compareGroupUserIDs bool) bool { func isEqualUserGroupPair(l, r *ec2.UserIdGroupPair, compareGroupUserIDs bool) bool {
glog.V(2).Infof("Comparing %v to %v", *l.GroupId, *r.GroupId)
if isEqualStringPointer(l.GroupId, r.GroupId) { if isEqualStringPointer(l.GroupId, r.GroupId) {
if compareGroupUserIDs { if compareGroupUserIDs {
if isEqualStringPointer(l.UserId, r.UserId) { if isEqualStringPointer(l.UserId, r.UserId) {
@ -1500,7 +1502,7 @@ func (s *AWSCloud) ensureSecurityGroupIngress(securityGroupId string, addPermiss
found := false found := false
for _, groupPermission := range group.IpPermissions { for _, groupPermission := range group.IpPermissions {
if isEqualIPPermission(addPermission, groupPermission, hasUserID) { if ipPermissionExists(addPermission, groupPermission, hasUserID) {
found = true found = true
break break
} }
@ -1555,8 +1557,8 @@ func (s *AWSCloud) removeSecurityGroupIngress(securityGroupId string, removePerm
var found *ec2.IpPermission var found *ec2.IpPermission
for _, groupPermission := range group.IpPermissions { for _, groupPermission := range group.IpPermissions {
if isEqualIPPermission(groupPermission, removePermission, hasUserID) { if ipPermissionExists(removePermission, groupPermission, hasUserID) {
found = groupPermission found = removePermission
break break
} }
} }

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

@ -800,7 +800,7 @@ func TestSubnetIDsinVPC(t *testing.T) {
} }
func TestIsEqualIPPermissionHandlesMultipleGroupIds(t *testing.T) { func TestIpPermissionExistsHandlesMultipleGroupIds(t *testing.T) {
oldIpPermission := ec2.IpPermission { oldIpPermission := ec2.IpPermission {
UserIdGroupPairs: []*ec2.UserIdGroupPair { UserIdGroupPairs: []*ec2.UserIdGroupPair {
&ec2.UserIdGroupPair{GroupId: aws.String("firstGroupId")}, &ec2.UserIdGroupPair{GroupId: aws.String("firstGroupId")},
@ -822,19 +822,18 @@ func TestIsEqualIPPermissionHandlesMultipleGroupIds(t *testing.T) {
} }
equals := isEqualIPPermission(&existingIpPermission, &oldIpPermission, false) equals := ipPermissionExists(&existingIpPermission, &oldIpPermission, false)
if !equals { if !equals {
t.Errorf("Should have been considered equal since first is in the second array of groups") t.Errorf("Should have been considered equal since first is in the second array of groups")
} }
equals = isEqualIPPermission(&newIpPermission, &oldIpPermission, false) equals = ipPermissionExists(&newIpPermission, &oldIpPermission, false)
if equals { if equals {
t.Errorf("Should have not been considered equal since first is not in the second array of groups") t.Errorf("Should have not been considered equal since first is not in the second array of groups")
} }
} }
func TestIsEqualIPPermissionHandlesMultipleGroupIdsWithUserIds(t *testing.T) { func TestIpPermissionExistsHandlesMultipleGroupIdsWithUserIds(t *testing.T) {
oldIpPermission := ec2.IpPermission { oldIpPermission := ec2.IpPermission {
UserIdGroupPairs: []*ec2.UserIdGroupPair { UserIdGroupPairs: []*ec2.UserIdGroupPair {
&ec2.UserIdGroupPair{GroupId: aws.String("firstGroupId"), UserId: aws.String("firstUserId")}, &ec2.UserIdGroupPair{GroupId: aws.String("firstGroupId"), UserId: aws.String("firstUserId")},
@ -856,12 +855,12 @@ func TestIsEqualIPPermissionHandlesMultipleGroupIdsWithUserIds(t *testing.T) {
} }
equals := isEqualIPPermission(&existingIpPermission, &oldIpPermission, true) equals := ipPermissionExists(&existingIpPermission, &oldIpPermission, true)
if !equals { if !equals {
t.Errorf("Should have been considered equal since first is in the second array of groups") t.Errorf("Should have been considered equal since first is in the second array of groups")
} }
equals = isEqualIPPermission(&newIpPermission, &oldIpPermission, true) equals = ipPermissionExists(&newIpPermission, &oldIpPermission, true)
if equals { if equals {
t.Errorf("Should have not been considered equal since first is not in the second array of groups") t.Errorf("Should have not been considered equal since first is not in the second array of groups")
} }