Merge pull request #56759 from aledbf/fix-nlb-icmp

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fix NLB icmp permission duplication

**What this PR does / why we need it**:

Fixes an issue with the ICMP rule for MTU during the creation of a NLB

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:

Fixes #56703
This commit is contained in:
Kubernetes Submit Queue 2018-01-09 12:20:29 -08:00 committed by GitHub
commit eff07b6f55
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -662,7 +662,6 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLBTraffic(actualGroups []*ec2.Se
if clientTraffic {
clientRuleAnnotation := fmt.Sprintf("%s=%s", NLBClientRuleDescription, lbName)
mtuRuleAnnotation := fmt.Sprintf("%s=%s", NLBMtuDiscoveryRuleDescription, lbName)
// Client Traffic
permission := &ec2.IpPermission{
FromPort: aws.Int64(port),
@ -682,26 +681,6 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLBTraffic(actualGroups []*ec2.Se
} else {
removes = append(removes, permission)
}
// MTU discovery
permission = &ec2.IpPermission{
IpProtocol: aws.String("icmp"),
FromPort: aws.Int64(3),
ToPort: aws.Int64(4),
}
ranges = []*ec2.IpRange{}
for _, cidr := range clientCidrs {
ranges = append(ranges, &ec2.IpRange{
CidrIp: aws.String(cidr),
Description: aws.String(mtuRuleAnnotation),
})
}
permission.IpRanges = ranges
if add {
adds = append(adds, permission)
} else {
removes = append(removes, permission)
}
} else {
healthRuleAnnotation := fmt.Sprintf("%s=%s", NLBHealthCheckRuleDescription, lbName)
@ -725,8 +704,8 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLBTraffic(actualGroups []*ec2.Se
removes = append(removes, permission)
}
}
}
if len(adds) > 0 {
changed, err := c.addSecurityGroupIngress(instanceSecurityGroupID, adds)
if err != nil {
@ -736,6 +715,7 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLBTraffic(actualGroups []*ec2.Se
glog.Warning("Allowing ingress was not needed; concurrent change? groupId=", instanceSecurityGroupID)
}
}
if len(removes) > 0 {
changed, err := c.removeSecurityGroupIngress(instanceSecurityGroupID, removes)
if err != nil {
@ -745,6 +725,70 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLBTraffic(actualGroups []*ec2.Se
glog.Warning("Revoking ingress was not needed; concurrent change? groupId=", instanceSecurityGroupID)
}
}
if clientTraffic {
// MTU discovery
mtuRuleAnnotation := fmt.Sprintf("%s=%s", NLBMtuDiscoveryRuleDescription, lbName)
mtuPermission := &ec2.IpPermission{
IpProtocol: aws.String("icmp"),
FromPort: aws.Int64(3),
ToPort: aws.Int64(4),
}
ranges := []*ec2.IpRange{}
for _, cidr := range clientCidrs {
ranges = append(ranges, &ec2.IpRange{
CidrIp: aws.String(cidr),
Description: aws.String(mtuRuleAnnotation),
})
}
mtuPermission.IpRanges = ranges
group, err := c.findSecurityGroup(instanceSecurityGroupID)
if err != nil {
glog.Warningf("Error retrieving security group: %q", err)
return err
}
if group == nil {
glog.Warning("Security group not found: ", instanceSecurityGroupID)
return nil
}
icmpExists := false
permCount := 0
for _, perm := range group.IpPermissions {
if *perm.IpProtocol == "icmp" {
icmpExists = true
continue
}
if perm.FromPort != nil {
permCount++
}
}
if !icmpExists && permCount > 0 {
// the icmp permission is missing
changed, err := c.addSecurityGroupIngress(instanceSecurityGroupID, []*ec2.IpPermission{mtuPermission})
if err != nil {
glog.Warningf("Error adding MTU permission to security group: %q", err)
return err
}
if !changed {
glog.Warning("Allowing ingress was not needed; concurrent change? groupId=", instanceSecurityGroupID)
}
} else if icmpExists && permCount == 0 {
// there is no additional permissions, remove icmp
changed, err := c.removeSecurityGroupIngress(instanceSecurityGroupID, []*ec2.IpPermission{mtuPermission})
if err != nil {
glog.Warningf("Error removing MTU permission to security group: %q", err)
return err
}
if !changed {
glog.Warning("Revoking ingress was not needed; concurrent change? groupId=", instanceSecurityGroupID)
}
}
}
}
return nil
}