Merge pull request #487 from nilo19/feature/deny-rule

feat: Add service annotation `ServiceAnnotationDenyAllExpectSourceRanges`
kubernetes-sigs · Jan 23, 2021 · 49705b6 · 49705b6
2 parents 2986f38 + b559df7
commit 49705b6
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 0 deletions.
diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go
@@ -90,6 +90,10 @@ const (
 	// Refer https://docs.microsoft.com/en-us/azure/virtual-network/security-overview#service-tags for all supported service tags.
 	ServiceAnnotationAllowedServiceTag = "service.beta.kubernetes.io/azure-allowed-service-tags"
 
+	// ServiceAnnotationDenyAllExceptLoadBalancerSourceRanges  denies all traffic to the load balancer except those
+	// within the service.Spec.LoadBalancerSourceRanges. Ref: https://github.com/kubernetes-sigs/cloud-provider-azure/issues/374.
+	ServiceAnnotationDenyAllExceptLoadBalancerSourceRanges = "service.beta.kubernetes.io/azure-deny-all-except-load-balancer-source-ranges"
+
 	// ServiceAnnotationLoadBalancerIdleTimeout is the annotation used on the service
 	// to specify the idle timeout for connections on the load balancer in minutes.
 	ServiceAnnotationLoadBalancerIdleTimeout = "service.beta.kubernetes.io/azure-load-balancer-tcp-idle-timeout"
@@ -1909,6 +1913,34 @@ func (az *Cloud) reconcileSecurityGroup(clusterName string, service *v1.Service,
 				}
 			}
 		}
+
+		shouldAddDenyRule := false
+		if len(sourceRanges) > 0 {
+			if v, ok := service.Annotations[ServiceAnnotationDenyAllExceptLoadBalancerSourceRanges]; ok && strings.EqualFold(v, "true") {
+				shouldAddDenyRule = true
+			}
+		}
+		if shouldAddDenyRule {
+			for _, port := range ports {
+				_, securityProto, _, err := getProtocolsFromKubernetesProtocol(port.Protocol)
+				if err != nil {
+					return nil, err
+				}
+				securityRuleName := az.getSecurityRuleName(service, port, "deny_all")
+				expectedSecurityRules = append(expectedSecurityRules, network.SecurityRule{
+					Name: to.StringPtr(securityRuleName),
+					SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{
+						Protocol:                 *securityProto,
+						SourcePortRange:          to.StringPtr("*"),
+						DestinationPortRange:     to.StringPtr(strconv.Itoa(int(port.Port))),
+						SourceAddressPrefix:      to.StringPtr("*"),
+						DestinationAddressPrefix: to.StringPtr(destinationIPAddress),
+						Access:                   network.SecurityRuleAccessDeny,
+						Direction:                network.SecurityRuleDirectionInbound,
+					},
+				})
+			}
+		}
 	}
 
 	for _, r := range expectedSecurityRules {

diff --git a/pkg/provider/azure_loadbalancer_test.go b/pkg/provider/azure_loadbalancer_test.go
@@ -2444,6 +2444,61 @@ func TestReconcileSecurityGroup(t *testing.T) {
 	}
 }
 
+func TestReconcileSecurityGroupLoadBalancerSourceRanges(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	az := GetTestCloud(ctrl)
+	service := getTestService("test1", v1.ProtocolTCP, map[string]string{ServiceAnnotationDenyAllExceptLoadBalancerSourceRanges: "true"}, false, 80)
+	service.Spec.LoadBalancerSourceRanges = []string{"1.2.3.4/32"}
+	existingSg := network.SecurityGroup{
+		Name: to.StringPtr("nsg"),
+		SecurityGroupPropertiesFormat: &network.SecurityGroupPropertiesFormat{
+			SecurityRules: &[]network.SecurityRule{},
+		},
+	}
+	lbIP := to.StringPtr("1.1.1.1")
+	expectedSg := network.SecurityGroup{
+		Name: to.StringPtr("nsg"),
+		SecurityGroupPropertiesFormat: &network.SecurityGroupPropertiesFormat{
+			SecurityRules: &[]network.SecurityRule{
+				{
+					Name: to.StringPtr("atest1-TCP-80-1.2.3.4_32"),
+					SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{
+						Protocol:                 network.SecurityRuleProtocol("Tcp"),
+						SourcePortRange:          to.StringPtr("*"),
+						SourceAddressPrefix:      to.StringPtr("1.2.3.4/32"),
+						DestinationPortRange:     to.StringPtr("80"),
+						DestinationAddressPrefix: to.StringPtr("1.1.1.1"),
+						Access:                   network.SecurityRuleAccess("Allow"),
+						Priority:                 to.Int32Ptr(500),
+						Direction:                network.SecurityRuleDirection("Inbound"),
+					},
+				},
+				{
+					Name: to.StringPtr("atest1-TCP-80-deny_all"),
+					SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{
+						Protocol:                 network.SecurityRuleProtocol("Tcp"),
+						SourcePortRange:          to.StringPtr("*"),
+						SourceAddressPrefix:      to.StringPtr("*"),
+						DestinationPortRange:     to.StringPtr("80"),
+						DestinationAddressPrefix: to.StringPtr("1.1.1.1"),
+						Access:                   network.SecurityRuleAccess("Deny"),
+						Priority:                 to.Int32Ptr(501),
+						Direction:                network.SecurityRuleDirection("Inbound"),
+					},
+				},
+			},
+		},
+	}
+	mockSGClient := az.SecurityGroupsClient.(*mocksecuritygroupclient.MockInterface)
+	mockSGClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, gomock.Any(), gomock.Any()).Return(existingSg, nil)
+	mockSGClient.EXPECT().CreateOrUpdate(gomock.Any(), az.ResourceGroup, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
+	sg, err := az.reconcileSecurityGroup("testCluster", &service, lbIP, true)
+	assert.NoError(t, err)
+	assert.Equal(t, expectedSg, *sg)
+}
+
 func TestSafeDeletePublicIP(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()