From 40d2dd79574f14a2e9d15fea877b30fe55894304 Mon Sep 17 00:00:00 2001 From: qini Date: Mon, 29 Jun 2020 16:45:31 +0800 Subject: [PATCH] Delete default load balancer source range (0.0.0.0/0) to prevent redundant network security rules. --- .../azure/azure_loadbalancer.go | 9 ++++ .../azure/azure_loadbalancer_test.go | 42 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go index 8c56e5c5a73..e12b9accb50 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go @@ -102,6 +102,8 @@ const ( serviceTagKey = "service" // clusterNameKey is the cluster name key applied for public IP tags. clusterNameKey = "kubernetes-cluster-name" + + defaultLoadBalancerSourceRanges = "0.0.0.0/0" ) // GetLoadBalancer returns whether the specified load balancer and its components exist, and @@ -1130,6 +1132,7 @@ func (az *Cloud) reconcileSecurityGroup(clusterName string, service *v1.Service, if lbIP != nil { destinationIPAddress = *lbIP } + if destinationIPAddress == "" { destinationIPAddress = "*" } @@ -1139,6 +1142,12 @@ func (az *Cloud) reconcileSecurityGroup(clusterName string, service *v1.Service, return nil, err } serviceTags := getServiceTags(service) + if len(serviceTags) != 0 { + if _, ok := sourceRanges[defaultLoadBalancerSourceRanges]; ok { + delete(sourceRanges, defaultLoadBalancerSourceRanges) + } + } + var sourceAddressPrefixes []string if (sourceRanges == nil || servicehelpers.IsAllowAll(sourceRanges)) && len(serviceTags) == 0 { if !requiresInternalLoadBalancer(service) { diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go index 4f50561dfeb..ceb249d3e12 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go @@ -1891,6 +1891,48 @@ func TestReconcileSecurityGroup(t *testing.T) { }, }, }, + { + desc: "reconcileSecurityGroup shall not create unwanted security rules if there is service tags", + service: getTestService("test1", v1.ProtocolTCP, map[string]string{ServiceAnnotationAllowedServiceTag: "tag"}, true, 80), + wantLb: true, + lbIP: to.StringPtr("1.1.1.1"), + existingSgs: map[string]network.SecurityGroup{"nsg": { + Name: to.StringPtr("nsg"), + SecurityGroupPropertiesFormat: &network.SecurityGroupPropertiesFormat{ + SecurityRules: &[]network.SecurityRule{ + { + Name: to.StringPtr("atest1-toBeDeleted"), + SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{ + SourceAddressPrefix: to.StringPtr("prefix"), + SourcePortRange: to.StringPtr("range"), + DestinationAddressPrefix: to.StringPtr("desPrefix"), + DestinationPortRange: to.StringPtr("desRange"), + }, + }, + }, + }, + }}, + expectedSg: &network.SecurityGroup{ + Name: to.StringPtr("nsg"), + SecurityGroupPropertiesFormat: &network.SecurityGroupPropertiesFormat{ + SecurityRules: &[]network.SecurityRule{ + { + Name: to.StringPtr("atest1-TCP-80-tag"), + SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{ + Protocol: network.SecurityRuleProtocol("Tcp"), + SourcePortRange: to.StringPtr("*"), + DestinationPortRange: to.StringPtr("80"), + SourceAddressPrefix: to.StringPtr("tag"), + DestinationAddressPrefix: to.StringPtr("1.1.1.1"), + Access: network.SecurityRuleAccess("Allow"), + Priority: to.Int32Ptr(500), + Direction: network.SecurityRuleDirection("Inbound"), + }, + }, + }, + }, + }, + }, } for i, test := range testCases {