add flag to set nodebalancer subnet and make sure nb backend ips lie within that subnet

Author: Rahul Sharma

cloud/linode/cloud.go

@@ -39,16 +39,17 @@ var Options struct {
 	EnableRouteController    bool
 	EnableTokenHealthChecker bool
 	// Deprecated: use VPCNames instead
-	VPCName               string
-	VPCNames              string
-	SubnetNames           string
-	LoadBalancerType      string
-	BGPNodeSelector       string
-	IpHolderSuffix        string
-	LinodeExternalNetwork *net.IPNet
-	NodeBalancerTags      []string
-	DefaultNBType         string
-	GlobalStopChannel     chan<- struct{}
+	VPCName                       string
+	VPCNames                      string
+	SubnetNames                   string
+	LoadBalancerType              string
+	BGPNodeSelector               string
+	IpHolderSuffix                string
+	LinodeExternalNetwork         *net.IPNet
+	NodeBalancerTags              []string
+	DefaultNBType                 string
+	NodeBalancerBackendIPv4Subnet string
+	GlobalStopChannel             chan<- struct{}
 }
 
 type linodeCloud struct {

cloud/linode/loadbalancers.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"net"
 	"net/http"
 	"os"
 	"reflect"
@@ -384,8 +385,11 @@ func (l *loadbalancers) updateNodeBalancer(
 		// Add all of the Nodes to the config
 		newNBNodes := make([]linodego.NodeBalancerConfigRebuildNodeOptions, 0, len(nodes))
 		subnetID := 0
-		_, ok := service.GetAnnotations()[annotations.NodeBalancerBackendIPv4Range]
+		backendIPv4Range, ok := service.GetAnnotations()[annotations.NodeBalancerBackendIPv4Range]
 		if ok {
+			if err := validateNodeBalancerBackendIPv4Range(backendIPv4Range); err != nil {
+				return err
+			}
 			id, err := l.getSubnetIDForSVC(ctx, service)
 			if err != nil {
 				sentry.CaptureError(ctx, err)
@@ -664,6 +668,9 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri
 
 	backendIPv4Range, ok := service.GetAnnotations()[annotations.NodeBalancerBackendIPv4Range]
 	if ok {
+		if err := validateNodeBalancerBackendIPv4Range(backendIPv4Range); err != nil {
+			return nil, err
+		}
 		subnetID, err := l.getSubnetIDForSVC(ctx, service)
 		if err != nil {
 			return nil, err
@@ -824,8 +831,11 @@ func (l *loadbalancers) buildLoadBalancerRequest(ctx context.Context, clusterNam
 	configs := make([]*linodego.NodeBalancerConfigCreateOptions, 0, len(ports))
 
 	subnetID := 0
-	_, ok := service.GetAnnotations()[annotations.NodeBalancerBackendIPv4Range]
+	backendIPv4Range, ok := service.GetAnnotations()[annotations.NodeBalancerBackendIPv4Range]
 	if ok {
+		if err := validateNodeBalancerBackendIPv4Range(backendIPv4Range); err != nil {
+			return nil, err
+		}
 		id, err := l.getSubnetIDForSVC(ctx, service)
 		if err != nil {
 			return nil, err
@@ -1086,3 +1096,32 @@ func getServiceBoolAnnotation(service *v1.Service, name string) bool {
 	boolValue, err := strconv.ParseBool(value)
 	return err == nil && boolValue
 }
+
+// validateNodeBalancerBackendIPv4Range validates the NodeBalancerBackendIPv4Range
+// annotation to be within the NodeBalancerBackendIPv4Subnet if it is set.
+func validateNodeBalancerBackendIPv4Range(backendIPv4Range string) error {
+	if Options.NodeBalancerBackendIPv4Subnet == "" {
+		return nil
+	}
+	withinCIDR, err := isCIDRWithinCIDR(Options.NodeBalancerBackendIPv4Subnet, backendIPv4Range)
+	if err != nil {
+		return fmt.Errorf("invalid IPv4 range: %v", err)
+	}
+	if !withinCIDR {
+		return fmt.Errorf("IPv4 range %s is not within the subnet %s", backendIPv4Range, Options.NodeBalancerBackendIPv4Subnet)
+	}
+	return nil
+}
+
+// isCIDRWithinCIDR returns true if the inner CIDR is within the outer CIDR.
+func isCIDRWithinCIDR(outer, inner string) (bool, error) {
+	_, ipNet1, err := net.ParseCIDR(outer)
+	if err != nil {
+		return false, fmt.Errorf("invalid CIDR: %v", err)
+	}
+	_, ipNet2, err := net.ParseCIDR(inner)
+	if err != nil {
+		return false, fmt.Errorf("invalid CIDR: %v", err)
+	}
+	return ipNet1.Contains(ipNet2.IP), nil
+}

cloud/linode/loadbalancers_test.go

@@ -156,6 +156,10 @@ func TestCCMLoadBalancers(t *testing.T) {
 			name: "Create Load Balancer With VPC Backend",
 			f:    testCreateNodeBalancerWithVPCBackend,
 		},
+		{
+			name: "Update Load Balancer With VPC Backend",
+			f:    testUpdateNodeBalancerWithVPCBackend,
+		},
 		{
 			name: "Create Load Balancer With VPC Backend - Overwrite VPC Name and Subnet with Annotation",
 			f:    testCreateNodeBalancerWithVPCAnnotationOverwrite,
@@ -525,15 +529,110 @@ func testCreateNodeBalancerWithVPCBackend(t *testing.T, client *linodego.Client,
 	if err != nil {
 		t.Fatalf("expected a nil error, got %v", err)
 	}
+
+	f.ResetRequests()
+
+	// test with IPv4Range outside of defined NodeBalancer subnet
+	nodebalancerBackendIPv4Subnet := Options.NodeBalancerBackendIPv4Subnet
+	defer func() {
+		Options.NodeBalancerBackendIPv4Subnet = nodebalancerBackendIPv4Subnet
+	}()
+	Options.NodeBalancerBackendIPv4Subnet = "10.99.0.0/24"
+	if err := testCreateNodeBalancer(t, client, f, ann, nil); err == nil {
+		t.Fatalf("expected nodebalancer creation to fail")
+	}
+}
+
+func testUpdateNodeBalancerWithVPCBackend(t *testing.T, client *linodego.Client, f *fakeAPI) {
+	// provision vpc and test
+	vpcNames := Options.VPCNames
+	subnetNames := Options.SubnetNames
+	defer func() {
+		Options.VPCNames = vpcNames
+		Options.SubnetNames = subnetNames
+	}()
+	Options.VPCNames = "test1"
+	Options.SubnetNames = "default"
+	_, _ = client.CreateVPC(context.TODO(), linodego.VPCCreateOptions{
+		Label:       "test1",
+		Description: "",
+		Region:      "us-west",
+		Subnets: []linodego.VPCSubnetCreateOptions{
+			{
+				Label: "default",
+				IPv4:  "10.0.0.0/8",
+			},
+		},
+	})
+
+	svc := &v1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: randString(),
+			UID:  "foobar123",
+			Annotations: map[string]string{
+				annotations.NodeBalancerBackendIPv4Range: "10.100.0.0/30",
+			},
+		},
+		Spec: v1.ServiceSpec{
+			Ports: []v1.ServicePort{
+				{
+					Name:     randString(),
+					Protocol: "TCP",
+					Port:     int32(80),
+					NodePort: int32(30000),
+				},
+			},
+		},
+	}
+
+	nodes := []*v1.Node{
+		{
+			Status: v1.NodeStatus{
+				Addresses: []v1.NodeAddress{
+					{
+						Type:    v1.NodeInternalIP,
+						Address: "127.0.0.1",
+					},
+				},
+			},
+		},
+	}
+
+	lb := newLoadbalancers(client, "us-west").(*loadbalancers)
+	fakeClientset := fake.NewSimpleClientset()
+	lb.kubeClient = fakeClientset
+
+	defer func() {
+		_ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc)
+	}()
+
+	lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes)
+	if err != nil {
+		t.Errorf("EnsureLoadBalancer returned an error: %s", err)
+	}
+	svc.Status.LoadBalancer = *lbStatus
+
+	stubService(fakeClientset, svc)
+	svc.ObjectMeta.SetAnnotations(map[string]string{
+		annotations.NodeBalancerBackendIPv4Range: "10.100.1.0/30",
+	})
+
+	err = lb.UpdateLoadBalancer(context.TODO(), "linodelb", svc, nodes)
+	if err != nil {
+		t.Errorf("UpdateLoadBalancer returned an error while updated annotations: %s", err)
+	}
 }
 
 func testCreateNodeBalancerWithVPCAnnotationOverwrite(t *testing.T, client *linodego.Client, f *fakeAPI) {
 	// provision multiple vpcs
 	vpcNames := Options.VPCNames
+	nodebalancerBackendIPv4Subnet := Options.NodeBalancerBackendIPv4Subnet
 	defer func() {
 		Options.VPCNames = vpcNames
+		Options.NodeBalancerBackendIPv4Subnet = nodebalancerBackendIPv4Subnet
 	}()
 	Options.VPCNames = "test1"
+	Options.NodeBalancerBackendIPv4Subnet = "10.100.0.0/24"
 
 	_, _ = client.CreateVPC(context.TODO(), linodego.VPCCreateOptions{
 		Label:       "test1",
@@ -3887,3 +3986,39 @@ func Test_loadbalancers_GetLinodeNBType(t *testing.T) {
 		})
 	}
 }
+
+func Test_validateNodeBalancerBackendIPv4Range(t *testing.T) {
+	type args struct {
+		backendIPv4Range string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name:    "Valid IPv4 range",
+			args:    args{backendIPv4Range: "10.100.0.0/30"},
+			wantErr: false,
+		},
+		{
+			name:    "Invalid IPv4 range",
+			args:    args{backendIPv4Range: "10.100.0.0"},
+			wantErr: true,
+		},
+	}
+
+	nbBackendSubnet := Options.NodeBalancerBackendIPv4Subnet
+	defer func() {
+		Options.NodeBalancerBackendIPv4Subnet = nbBackendSubnet
+	}()
+	Options.NodeBalancerBackendIPv4Subnet = "10.100.0.0/24"
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := validateNodeBalancerBackendIPv4Range(tt.args.backendIPv4Range); (err != nil) != tt.wantErr {
+				t.Errorf("validateNodeBalancerBackendIPv4Range() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}

deploy/chart/templates/daemonset.yaml

@@ -100,6 +100,9 @@ spec:
             {{- if .Values.defaultNBType }}
             - --default-nodebalancer-type={{ .Values.defaultNBType }}
             {{- end }}
+            {{- if .Values.nodeBalancerBackendIPv4Subnet }}
+            - --nodebalancer-backend-ipv4-subnet={{ .Values.nodeBalancerBackendIPv4Subnet }}
+            {{- end }}
           {{- with .Values.containerSecurityContext }}
           securityContext:
           {{- toYaml . | nindent 12 }}

deploy/chart/values.yaml

@@ -86,6 +86,9 @@ tolerations:
 # Default NodeBalancer type to create("common" or "premium"). Default is "common"
 # defaultNBType: "common"
 
+# nodeBalancerBackendIPv4Subnet is the subnet to use for the backend ips of the NodeBalancer
+# nodeBalancerBackendIPv4Subnet: ""
+
 # This section adds the ability to pass environment variables to adjust CCM defaults
 # https://github.com/linode/linode-cloud-controller-manager/blob/master/cloud/linode/loadbalancers.go
 # LINODE_HOSTNAME_ONLY_INGRESS type bool is supported

docs/configuration/loadbalancer.md

@@ -171,6 +171,8 @@ metadata:
     service.beta.kubernetes.io/linode-loadbalancer-subnet-name: "subnet1"
 ```
 
+If CCM is started with `--nodebalancer-backend-ipv4-subnet` flag, then it will not allow provisioning of nodebalancer unless subnet specified in service annotation lie within the subnet specified using the flag. This is to prevent accidental overlap between nodebalancer backend ips and pod CIDRs.
+
 ## Advanced Configuration
 
 ### Using Existing NodeBalancers

main.go

@@ -90,6 +90,7 @@ func main() {
 	command.Flags().StringVar(&linode.Options.BGPNodeSelector, "bgp-node-selector", "", "node selector to use to perform shared IP fail-over with BGP (e.g. cilium-bgp-peering=true")
 	command.Flags().StringVar(&linode.Options.IpHolderSuffix, "ip-holder-suffix", "", "suffix to append to the ip holder name when using shared IP fail-over with BGP (e.g. ip-holder-suffix=my-cluster-name")
 	command.Flags().StringVar(&linode.Options.DefaultNBType, "default-nodebalancer-type", string(linodego.NBTypeCommon), "default type of NodeBalancer to create (options: common, premium)")
+	command.Flags().StringVar(&linode.Options.NodeBalancerBackendIPv4Subnet, "nodebalancer-backend-ipv4-subnet", "", "ipv4 subnet to use for NodeBalancer backends")
 	command.Flags().StringSliceVar(&linode.Options.NodeBalancerTags, "nodebalancer-tags", []string{}, "Linode tags to apply to all NodeBalancers")
 
 	// Set static flags