Merge pull request #5124 from lodrem/feat-lb/access-control-ip-ranges…

…-release-1.25 [release-1.25] feat: Add annotation `service.beta.kubernetes.io/azure-allowed-ip-ranges`
kubernetes-sigs · Jan 16, 2024 · 6868d50 · 6868d50
2 parents 5ca827d + 264a6d0
commit 6868d50
Show file tree

Hide file tree

Showing 8 changed files with 1,274 additions and 26 deletions.
diff --git a/pkg/consts/consts.go b/pkg/consts/consts.go
@@ -277,10 +277,15 @@ const (
 	// ServiceAnnotationIPTagsForPublicIP specifies the iptags used when dynamically creating a public ip
 	ServiceAnnotationIPTagsForPublicIP = "service.beta.kubernetes.io/azure-pip-ip-tags"
 
-	// ServiceAnnotationAllowedServiceTag is the annotation used on the service
+	// ServiceAnnotationAllowedServiceTags is the annotation used on the service
 	// to specify a list of allowed service tags separated by comma
 	// 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"
+	ServiceAnnotationAllowedServiceTags = "service.beta.kubernetes.io/azure-allowed-service-tags"
+
+	// ServiceAnnotationAllowedIPRanges is the annotation used on the service
+	// to specify a list of allowed IP Ranges separated by comma.
+	// It is compatible with both IPv4 and IPV6 CIDR formats.
+	ServiceAnnotationAllowedIPRanges = "service.beta.kubernetes.io/azure-allowed-ip-ranges"
 
 	// 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.

diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go
@@ -45,6 +45,7 @@ import (
 	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
 	"sigs.k8s.io/cloud-provider-azure/pkg/metrics"
+	"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer"
 	"sigs.k8s.io/cloud-provider-azure/pkg/retry"
 )
 
@@ -2400,28 +2401,45 @@ func (az *Cloud) reconcileSecurityGroup(clusterName string, service *v1.Service,
 		destinationIPAddresses = append(destinationIPAddresses, additionalIPs...)
 	}
 
-	sourceRanges, err := servicehelpers.GetLoadBalancerSourceRanges(service)
+	accessControl, err := loadbalancer.NewAccessControl(service)
 	if err != nil {
+		klog.ErrorS(err, "Failed to parse access control configuration for service", "service", service.Name)
 		return nil, err
 	}
-	serviceTags := getServiceTags(service)
-	if len(serviceTags) != 0 {
-		delete(sourceRanges, consts.DefaultLoadBalancerSourceRanges)
-	}
 
-	var sourceAddressPrefixes []string
-	if (sourceRanges == nil || servicehelpers.IsAllowAll(sourceRanges)) && len(serviceTags) == 0 {
-		if !requiresInternalLoadBalancer(service) || len(service.Spec.LoadBalancerSourceRanges) > 0 {
-			sourceAddressPrefixes = []string{"Internet"}
-		}
-	} else {
-		for _, ip := range sourceRanges {
-			sourceAddressPrefixes = append(sourceAddressPrefixes, ip.String())
-		}
-		sourceAddressPrefixes = append(sourceAddressPrefixes, serviceTags...)
+	var (
+		sourceRanges          = accessControl.SourceRanges()
+		allowedServiceTags    = accessControl.AllowedServiceTags()
+		allowedIPRanges       = accessControl.AllowedIPRanges()
+		sourceAddressPrefixes = accessControl.IPV4Sources()
+	)
+
+	if len(sourceRanges) != 0 && len(allowedIPRanges) != 0 {
+		// Block the service and return error if both of spec.loadBalancerSourceRanges and annotation are specified
+		klog.Errorf("Service %s is using both of spec.loadBalancerSourceRanges and annotation %s.", service.Name, consts.ServiceAnnotationAllowedIPRanges)
+		return nil, fmt.Errorf(
+			"both of spec.loadBalancerSourceRanges and annotation %s are specified for service %s, which is not allowed",
+			consts.ServiceAnnotationAllowedIPRanges, service.Name,
+		)
+	}
+	if len(sourceRanges) != 0 && len(allowedServiceTags) != 0 {
+		// Suggesting to use aks custom annotation instead of spec.loadBalancerSourceRanges
+		klog.Warningf(
+			"Service %s is using both of spec.loadBalancerSourceRanges and annotation %s.",
+			service.Name, consts.ServiceAnnotationAllowedServiceTags,
+		)
+		az.Event(service, v1.EventTypeWarning, "ConflictConfiguration", fmt.Sprintf(
+			"Please use annotation %s instead of spec.loadBalancerSourceRanges while using %s annotation at the same time.",
+			consts.ServiceAnnotationAllowedIPRanges, consts.ServiceAnnotationAllowedServiceTags,
+		))
 	}
 
-	expectedSecurityRules, err := az.getExpectedSecurityRules(wantLb, ports, sourceAddressPrefixes, service, destinationIPAddresses, sourceRanges, backendIPAddresses, disableFloatingIP)
+	expectedSecurityRules, err := az.getExpectedSecurityRules(
+		wantLb, ports,
+		sourceAddressPrefixes, service,
+		destinationIPAddresses, sourceRanges,
+		backendIPAddresses, disableFloatingIP,
+	)
 	if err != nil {
 		return nil, err
 	}
@@ -2575,7 +2593,16 @@ func (az *Cloud) reconcileSecurityRules(sg network.SecurityGroup, service *v1.Se
 	return dirtySg, updatedRules, nil
 }
 
-func (az *Cloud) getExpectedSecurityRules(wantLb bool, ports []v1.ServicePort, sourceAddressPrefixes []string, service *v1.Service, destinationIPAddresses []string, sourceRanges utilnet.IPNetSet, backendIPAddresses []string, disableFloatingIP bool) ([]network.SecurityRule, error) {
+func (az *Cloud) getExpectedSecurityRules(
+	wantLb bool,
+	ports []v1.ServicePort,
+	sourceAddressPrefixes []string,
+	service *v1.Service,
+	destinationIPAddresses []string,
+	sourceRanges []netip.Prefix,
+	backendIPAddresses []string,
+	disableFloatingIP bool,
+) ([]network.SecurityRule, error) {
 	expectedSecurityRules := []network.SecurityRule{}
 
 	if wantLb {
@@ -2618,7 +2645,7 @@ func (az *Cloud) getExpectedSecurityRules(wantLb bool, ports []v1.ServicePort, s
 		}
 
 		shouldAddDenyRule := false
-		if len(sourceRanges) > 0 && !servicehelpers.IsAllowAll(sourceRanges) {
+		if len(sourceRanges) > 0 && !loadbalancer.IsCIDRsAllowAll(sourceRanges) {
 			if v, ok := service.Annotations[consts.ServiceAnnotationDenyAllExceptLoadBalancerSourceRanges]; ok && strings.EqualFold(v, consts.TrueAnnotationValue) {
 				shouldAddDenyRule = true
 			}
@@ -3327,7 +3354,7 @@ func getServiceTags(service *v1.Service) []string {
 		return nil
 	}
 
-	if serviceTags, found := service.Annotations[consts.ServiceAnnotationAllowedServiceTag]; found {
+	if serviceTags, found := service.Annotations[consts.ServiceAnnotationAllowedServiceTags]; found {
 		result := []string{}
 		tags := strings.Split(strings.TrimSpace(serviceTags), ",")
 		for _, tag := range tags {