Add support for IPv6 values in loadBalancerSourceRanges

pkg/loadbalancers/l4.go

@@ -482,15 +482,15 @@ func (l4 *L4) ensureIPv4NodesFirewall(nodeNames []string, ipAddress string, resu
 	}()
 
 	// ensure firewalls
-	sourceRanges, err := utils.ServiceSourceRanges(l4.Service)
+	ipv4SourceRanges, err := utils.IPv4ServiceSourceRanges(l4.Service)
 	if err != nil {
 		result.Error = err
 		return
 	}
 	// Add firewall rule for ILB traffic to nodes
 	nodesFWRParams := firewalls.FirewallParams{
 		PortRanges:        portRanges,
-		SourceRanges:      sourceRanges,
+		SourceRanges:      ipv4SourceRanges,
 		DestinationRanges: []string{ipAddress},
 		Protocol:          string(protocol),
 		Name:              firewallName,

pkg/loadbalancers/l4_test.go

@@ -2155,9 +2155,9 @@ func verifyILBIPv4NodesFirewall(l4 *L4, nodeNames []string) error {
 		return fmt.Errorf("failed to create description for resources, err %w", err)
 	}
 
-	sourceRanges, err := utils.ServiceSourceRanges(l4.Service)
+	sourceRanges, err := utils.IPv4ServiceSourceRanges(l4.Service)
 	if err != nil {
-		return fmt.Errorf("servicehelper.GetLoadBalancerSourceRanges(%+v) returned error %v, want nil", l4.Service, err)
+		return fmt.Errorf("utils.IPv4ServiceSourceRanges(%+v) returned error %v, want nil", l4.Service, err)
 	}
 	return verifyFirewall(l4.cloud, nodeNames, fwName, fwDesc, sourceRanges)
 }
@@ -2170,7 +2170,11 @@ func verifyILBIPv6NodesFirewall(l4 *L4, nodeNames []string) error {
 		return fmt.Errorf("failed to create description for resources, err %w", err)
 	}
 
-	return verifyFirewall(l4.cloud, nodeNames, ipv6FirewallName, fwDesc, []string{"0::0/0"})
+	sourceRanges, err := utils.IPv6ServiceSourceRanges(l4.Service)
+	if err != nil {
+		return fmt.Errorf("utils.IPv6ServiceSourceRanges(%+v) returned error %v, want nil", l4.Service, err)
+	}
+	return verifyFirewall(l4.cloud, nodeNames, ipv6FirewallName, fwDesc, sourceRanges)
 }
 
 func verifyILBIPv4HealthCheckFirewall(l4 *L4, nodeNames []string) error {

pkg/loadbalancers/l4ipv6.go

@@ -130,18 +130,24 @@ func (l4 *L4) ensureIPv6NodesFirewall(forwardingRule *composite.ForwardingRule,
 		klog.V(2).Infof("Finished ensuring IPv6 nodes firewall %s for L4 ILB Service %s/%s, time taken: %v", l4.Service.Namespace, l4.Service.Name, firewallName, time.Since(start))
 	}()
 
+	// ensure firewalls
+	ipv6SourceRanges, err := utils.IPv6ServiceSourceRanges(l4.Service)
+	if err != nil {
+		result.Error = err
+		return
+	}
+
 	ipv6nodesFWRParams := firewalls.FirewallParams{
-		PortRanges: portRanges,
-		// TODO(panslava): support .spec.loadBalancerSourceRanges
-		SourceRanges:      []string{"0::0/0"},
+		PortRanges:        portRanges,
+		SourceRanges:      ipv6SourceRanges,
 		DestinationRanges: []string{forwardingRule.IPAddress},
 		Protocol:          string(protocol),
 		Name:              firewallName,
 		NodeNames:         nodeNames,
 		L4Type:            utils.ILB,
 	}
 
-	err := firewalls.EnsureL4LBFirewallForNodes(l4.Service, &ipv6nodesFWRParams, l4.cloud, l4.recorder)
+	err = firewalls.EnsureL4LBFirewallForNodes(l4.Service, &ipv6nodesFWRParams, l4.cloud, l4.recorder)
 	if err != nil {
 		result.GCEResourceInError = annotations.FirewallRuleIPv6Resource
 		result.Error = err

pkg/loadbalancers/l4netlb.go

@@ -284,7 +284,7 @@ func (l4netlb *L4NetLB) ensureNodesFirewall(name string, nodeNames []string, ipA
 	}()
 
 	result := &L4NetLBSyncResult{}
-	sourceRanges, err := utils.ServiceSourceRanges(l4netlb.Service)
+	sourceRanges, err := utils.IPv4ServiceSourceRanges(l4netlb.Service)
 	if err != nil {
 		result.Error = err
 		return result

pkg/loadbalancers/l4netlb_test.go

@@ -331,7 +331,7 @@ func verifyNetLBNodesFirewall(l4netlb *L4NetLB, nodeNames []string) error {
 		return fmt.Errorf("failed to create description for resources, err %w", err)
 	}
 
-	sourceRanges, err := utils.ServiceSourceRanges(l4netlb.Service)
+	sourceRanges, err := utils.IPv4ServiceSourceRanges(l4netlb.Service)
 	if err != nil {
 		return fmt.Errorf("servicehelper.GetLoadBalancerSourceRanges(%+v) returned error %v, want nil", l4netlb.Service, err)
 	}

pkg/utils/sourceranges.go

@@ -10,18 +10,43 @@ import (
 
 const (
 	allowAllIPv4Range = "0.0.0.0/0"
+	allowAllIPv6Range = "0::0/0"
 )
 
-func ServiceSourceRanges(service *v1.Service) ([]string, error) {
-	ipRanges, err := getAllSourceRanges(service)
+func IPv4ServiceSourceRanges(service *v1.Service) ([]string, error) {
+	allRanges, err := getAllSourceRanges(service)
 	if err != nil {
 		return nil, err
 	}
 
-	if len(ipRanges) == 0 {
+	var ipv4Ranges []string
+	for _, ip := range allRanges {
+		if net.IsIPv4CIDR(ip) {
+			ipv4Ranges = append(ipv4Ranges, ip.String())
+		}
+	}
+	if len(ipv4Ranges) == 0 {
 		return []string{allowAllIPv4Range}, nil
 	}
-	return ipRanges.StringSlice(), nil
+	return ipv4Ranges, nil
+}
+
+func IPv6ServiceSourceRanges(service *v1.Service) ([]string, error) {
+	allRanges, err := getAllSourceRanges(service)
+	if err != nil {
+		return nil, err
+	}
+
+	var ipv6Ranges []string
+	for _, ip := range allRanges {
+		if net.IsIPv6CIDR(ip) {
+			ipv6Ranges = append(ipv6Ranges, ip.String())
+		}
+	}
+	if len(ipv6Ranges) == 0 {
+		return []string{allowAllIPv6Range}, nil
+	}
+	return ipv6Ranges, nil
 }
 
 func getAllSourceRanges(service *v1.Service) (net.IPNetSet, error) {

pkg/utils/sourceranges_test.go

@@ -10,37 +10,42 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func TestServiceSourceRanges(t *testing.T) {
+func TestIPv4ServiceSourceRanges(t *testing.T) {
 	testCases := []struct {
-		desc                 string
-		specRanges           []string
-		annotations          map[string]string
-		expectedSourceRanges []string
-		expectError          error
+		desc                     string
+		specRanges               []string
+		annotations              map[string]string
+		expectedIPv4SourceRanges []string
+		expectError              error
 	}{
 		{
-			desc:                 "Should return allow all for no specs or annotations",
-			expectedSourceRanges: []string{"0.0.0.0/0"},
+			desc:                     "Should return allow all for no specs or annotations",
+			expectedIPv4SourceRanges: []string{"0.0.0.0/0"},
 		},
 		{
-			desc:                 "Should parse ranges from spec",
-			specRanges:           []string{" 192.168.0.1/10", " 132.8.0.1/8 "},
-			expectedSourceRanges: []string{"192.128.0.0/10", "132.0.0.0/8"}, // only significant bits are left, spaces are trimmed
+			desc:                     "Should return allow all if only IPv6 CIDRs in Spec",
+			specRanges:               []string{" 0::0/0 ", "2001:db8:3333:4444:5555:6666:7777:8888/32"},
+			expectedIPv4SourceRanges: []string{"0.0.0.0/0"},
 		},
 		{
-			desc: "Should parse ranges from annotations, if no spec value",
+			desc:                     "Should get IPv4 addresses only, if mixed with IPv6",
+			specRanges:               []string{"0::0/0", " 192.168.0.1/10 ", "2001:db8:3333:4444:5555:6666:7777:8888/32", " 132.8.0.1/8"},
+			expectedIPv4SourceRanges: []string{"192.128.0.0/10", "132.0.0.0/8"}, // only significant bits are left, space are trimmed
+		},
+		{
+			desc: "Should get IPv4 addresses from annotation, if mixed with IPv6 and no spec ranges",
 			annotations: map[string]string{
-				v1.AnnotationLoadBalancerSourceRangesKey: " 192.168.0.1/10 , 132.8.0.1/8 ",
+				v1.AnnotationLoadBalancerSourceRangesKey: "0::0/0, 192.168.0.1/10 ,2001:db8:3333:4444:5555:6666:7777:8888/32, 132.8.0.1/8 ",
 			},
-			expectedSourceRanges: []string{"192.128.0.0/10", "132.0.0.0/8"}, // only significant bits are left, spaces are trimmed
+			expectedIPv4SourceRanges: []string{"192.128.0.0/10", "132.0.0.0/8"}, // only significant bits are left, spaces are trimmed
 		},
 		{
 			desc:       "Should ignore annotation if spec is present",
-			specRanges: []string{"192.168.0.1/10", "132.8.0.1/8"},
+			specRanges: []string{"0::0/0", " 192.168.0.1/10 ", "2001:db8:3333:4444:5555:6666:7777:8888/32 ", " 132.8.0.1/8"},
 			annotations: map[string]string{
 				v1.AnnotationLoadBalancerSourceRangesKey: "1.2.3 1.2.3", // should not return error, even if annotation is invalid
 			},
-			expectedSourceRanges: []string{"192.128.0.0/10", "132.0.0.0/8"}, // only significant bits are left
+			expectedIPv4SourceRanges: []string{"192.128.0.0/10", "132.0.0.0/8"}, // only significant bits are left, spaces are trimmed
 		},
 		{
 			desc:       "Should return special error for invalid spec value",
@@ -73,17 +78,101 @@ func TestServiceSourceRanges(t *testing.T) {
 				},
 			}
 
-			sourceRanges, err := ServiceSourceRanges(svc)
+			ipv4Ranges, err := IPv4ServiceSourceRanges(svc)
+			errDiff := cmp.Diff(tc.expectError, err)
+			if errDiff != "" {
+				t.Errorf("Expected error %v, got %v, diff: %v", tc.expectError, err, errDiff)
+			}
+
+			sort.Strings(tc.expectedIPv4SourceRanges)
+			sort.Strings(ipv4Ranges)
+			diff := cmp.Diff(tc.expectedIPv4SourceRanges, ipv4Ranges)
+			if diff != "" {
+				t.Errorf("Expected IPv4 ranges: %v, got ranges %v, diff: %s", tc.expectedIPv4SourceRanges, ipv4Ranges, diff)
+			}
+		})
+	}
+}
+
+func TestIPv6ServiceSourceRanges(t *testing.T) {
+	testCases := []struct {
+		desc                     string
+		specRanges               []string
+		annotations              map[string]string
+		expectedIPv6SourceRanges []string
+		expectError              error
+	}{
+		{
+			desc:                     "Should return allow all for no specs or annotations",
+			expectedIPv6SourceRanges: []string{"0::0/0"},
+		},
+		{
+			desc:                     "Should return allow all if only IPv4 CIDRs in Spec",
+			specRanges:               []string{" 1.2.3.4/5 ", " 192.168.0.1/10 "},
+			expectedIPv6SourceRanges: []string{"0::0/0"},
+		},
+		{
+			desc:                     "Should get IPv6 addresses only, if mixed with IPv4",
+			specRanges:               []string{" 2001:db8:4444:4444:5555:6666:7777:8888/10 ", " 192.168.0.1/10 ", "2001:db8:3333:4444:5555:6666:7777:8888/32", " 132.8.0.1/8"},
+			expectedIPv6SourceRanges: []string{"2000::/10", "2001:db8::/32"}, // only significant bits are left, space are trimmed
+		},
+		{
+			desc: "Should get IPv6 addresses from annotation, if mixed with IPv4 and no spec ranges",
+			annotations: map[string]string{
+				v1.AnnotationLoadBalancerSourceRangesKey: " 2001:db8:4444:4444:5555:6666:7777:8888/10 , 192.168.0.1/10 ,2001:db8:3333:4444:5555:6666:7777:8888/32, 132.8.0.1/8 ",
+			},
+			expectedIPv6SourceRanges: []string{"2000::/10", "2001:db8::/32"}, // only significant bits are left, space are trimmed
+		},
+		{
+			desc:       "Should ignore annotation if spec is present",
+			specRanges: []string{" 2001:db8:4444:4444:5555:6666:7777:8888/10 ", " 192.168.0.1/10 ", "2001:db8:3333:4444:5555:6666:7777:8888/32 ", " 132.8.0.1/8"},
+			annotations: map[string]string{
+				v1.AnnotationLoadBalancerSourceRangesKey: "2001:db8:3333:4444:5555:6666:7777:8888 1.2.3", // should not return error, even if annotation is invalid
+			},
+			expectedIPv6SourceRanges: []string{"2000::/10", "2001:db8::/32"}, // only significant bits are left, space are trimmed
+		},
+		{
+			desc:       "Should return special error for invalid spec value",
+			specRanges: []string{"2001:db8:4444:4444:5555:6666:7777:8888"}, // no mask
+			expectError: &InvalidLoadBalancerSourceRangesSpecError{
+				LoadBalancerSourceRangesSpec: []string{"2001:db8:4444:4444:5555:6666:7777:8888"},
+				ParseErr:                     &net.ParseError{Type: "CIDR address", Text: "2001:db8:4444:4444:5555:6666:7777:8888"},
+			},
+		},
+		{
+			desc: "Should return special error for invalid annotation value",
+			annotations: map[string]string{
+				v1.AnnotationLoadBalancerSourceRangesKey: "2001:db8:4444:4444:5555:6666:7777:8888/14 2001:db8:4444:4444:5555:6666:7777:8888/15", // should be comma-separated
+			},
+			expectError: &InvalidLoadBalancerSourceRangesAnnotationError{
+				LoadBalancerSourceRangesAnnotation: "2001:db8:4444:4444:5555:6666:7777:8888/14 2001:db8:4444:4444:5555:6666:7777:8888/15",
+				ParseErr:                           &net.ParseError{Type: "CIDR address", Text: "2001:db8:4444:4444:5555:6666:7777:8888/14 2001:db8:4444:4444:5555:6666:7777:8888/15"},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			svc := &v1.Service{
+				Spec: v1.ServiceSpec{
+					LoadBalancerSourceRanges: tc.specRanges,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: tc.annotations,
+				},
+			}
+
+			ipv6Ranges, err := IPv6ServiceSourceRanges(svc)
 			errDiff := cmp.Diff(tc.expectError, err)
 			if errDiff != "" {
 				t.Errorf("Expected error %v, got %v, diff: %v", tc.expectError, err, errDiff)
 			}
 
-			sort.Strings(tc.expectedSourceRanges)
-			sort.Strings(sourceRanges)
-			diff := cmp.Diff(tc.expectedSourceRanges, sourceRanges)
+			sort.Strings(tc.expectedIPv6SourceRanges)
+			sort.Strings(ipv6Ranges)
+			diff := cmp.Diff(tc.expectedIPv6SourceRanges, ipv6Ranges)
 			if diff != "" {
-				t.Errorf("Expected source ranges: %v, got ranges %v, diff: %s", tc.expectedSourceRanges, sourceRanges, diff)
+				t.Errorf("Expected IPv6 ranges: %v, got ranges %v, diff: %s", tc.expectedIPv6SourceRanges, ipv6Ranges, diff)
 			}
 		})
 	}