Verify LB ip and address pool annotations compatibility

When a service requests a specific LB IP(s) and address pool via annotations, these must be compatible. Fixes #1916 Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
metallb · May 3, 2023 · fa3184f · fa3184f
1 parent eb58887
commit fa3184f
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 9 deletions.
diff --git a/controller/controller_test.go b/controller/controller_test.go
@@ -320,6 +320,23 @@ func TestControllerMutation(t *testing.T) {
 			wantErr: true,
 		},
 
+		{
+			desc: "incompatible ip and address pool annotations",
+			in: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						annotationLoadBalancerIPs: "1.2.3.1",
+						annotationAddressPool:     "pool2",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					ClusterIPs: []string{"1.2.3.4"},
+					Type:       "LoadBalancer",
+				},
+			},
+			wantErr: true,
+		},
+
 		{
 			desc: "request invalid IP via custom annotation",
 			in: &v1.Service{
@@ -377,7 +394,7 @@ func TestControllerMutation(t *testing.T) {
 			in: &v1.Service{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						"metallb.universe.tf/address-pool": "pool1",
+						annotationAddressPool: "pool1",
 					},
 				},
 				Spec: v1.ServiceSpec{
@@ -388,7 +405,7 @@ func TestControllerMutation(t *testing.T) {
 			want: &v1.Service{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						"metallb.universe.tf/address-pool": "pool1",
+						annotationAddressPool: "pool1",
 					},
 				},
 				Spec: v1.ServiceSpec{
@@ -404,7 +421,7 @@ func TestControllerMutation(t *testing.T) {
 			in: &v1.Service{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						"metallb.universe.tf/address-pool": "pool2",
+						annotationAddressPool: "pool2",
 					},
 				},
 				Spec: v1.ServiceSpec{
@@ -416,7 +433,7 @@ func TestControllerMutation(t *testing.T) {
 			want: &v1.Service{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						"metallb.universe.tf/address-pool": "pool2",
+						annotationAddressPool: "pool2",
 					},
 				},
 				Spec: v1.ServiceSpec{
@@ -432,7 +449,7 @@ func TestControllerMutation(t *testing.T) {
 			in: &v1.Service{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						"metallb.universe.tf/address-pool": "does-not-exist",
+						annotationAddressPool: "does-not-exist",
 					},
 				},
 				Spec: v1.ServiceSpec{
@@ -673,7 +690,7 @@ func TestControllerMutation(t *testing.T) {
 			in: &v1.Service{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						"metallb.universe.tf/address-pool": "pool5",
+						annotationAddressPool: "pool5",
 					},
 				},
 				Spec: v1.ServiceSpec{
@@ -684,7 +701,7 @@ func TestControllerMutation(t *testing.T) {
 			want: &v1.Service{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						"metallb.universe.tf/address-pool": "pool5",
+						annotationAddressPool: "pool5",
 					},
 				},
 				Spec: v1.ServiceSpec{

diff --git a/controller/service.go b/controller/service.go
@@ -193,6 +193,8 @@ func (c *controller) allocateIPs(key string, svc *v1.Service) ([]net.IP, error)
 		return nil, err
 	}
 
+	desiredPool := svc.Annotations[annotationAddressPool]
+
 	// If the user asked for a specific IPs, try that.
 	if len(desiredLbIPs) > 0 {
 		if serviceIPFamily != desiredLbIPFamily {
@@ -201,10 +203,17 @@ func (c *controller) allocateIPs(key string, svc *v1.Service) ([]net.IP, error)
 		if err := c.ips.Assign(key, svc, desiredLbIPs, k8salloc.Ports(svc), k8salloc.SharingKey(svc), k8salloc.BackendKey(svc)); err != nil {
 			return nil, err
 		}
+
+		// Verify that ip and address pool annotations are compatible.
+		if desiredPool != "" && c.ips.Pool(key) != desiredPool {
+			c.ips.Unassign(key)
+			return nil, fmt.Errorf("requested loadBalancer IP(s) %q is not compatible with requested address pool %s", desiredLbIPs, desiredPool)
+		}
+
 		return desiredLbIPs, nil
 	}
-	// Otherwise, did the user ask for a specific pool?
-	desiredPool := svc.Annotations[annotationAddressPool]
+
+	// Assign ip from requested address pool.
 	if desiredPool != "" {
 		ips, err := c.ips.AllocateFromPool(key, svc, serviceIPFamily, desiredPool, k8salloc.Ports(svc), k8salloc.SharingKey(svc), k8salloc.BackendKey(svc))
 		if err != nil {