Merge pull request #9230 from fasaxc/fix-bgpconf-validation

Ignore empty CIDRs in BGPConfiguration
projectcalico · Sep 16, 2024 · 4ad72b7 · 4ad72b7
2 parents f9da6ec + ab71c83
commit 4ad72b7
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 9 deletions.
diff --git a/confd/pkg/backends/calico/client.go b/confd/pkg/backends/calico/client.go
@@ -1266,9 +1266,13 @@ func (c *client) getServiceExternalIPsKVPair(v3res *apiv3.BGPConfiguration, key
 	if v3res != nil && v3res.Spec.ServiceExternalIPs != nil && len(v3res.Spec.ServiceExternalIPs) != 0 {
 		// We wrap each Service external IP in a ServiceExternalIPBlock struct to
 		// achieve the desired API structure, unpack that.
-		ipCidrs := make([]string, len(v3res.Spec.ServiceExternalIPs))
-		for i, ipBlock := range v3res.Spec.ServiceExternalIPs {
-			ipCidrs[i] = ipBlock.CIDR
+		ipCidrs := make([]string, 0, len(v3res.Spec.ServiceExternalIPs))
+		for _, ipBlock := range v3res.Spec.ServiceExternalIPs {
+			if ipBlock.CIDR == "" {
+				// The CRD allows CIDR to be optional so we just ignore empty CIDRs.
+				continue
+			}
+			ipCidrs = append(ipCidrs, ipBlock.CIDR)
 		}
 		c.updateCache(api.UpdateTypeKVUpdated, getKVPair(svcExternalIPKey, strings.Join(ipCidrs, ",")))
 	} else {
@@ -1281,9 +1285,13 @@ func (c *client) getServiceLoadBalancerIPsKVPair(v3res *apiv3.BGPConfiguration,
 	svcLoadBalancerIPKey := getBGPConfigKey("svc_loadbalancer_ips", key)
 
 	if v3res != nil && v3res.Spec.ServiceLoadBalancerIPs != nil && len(v3res.Spec.ServiceLoadBalancerIPs) != 0 {
-		ipCidrs := make([]string, len(v3res.Spec.ServiceLoadBalancerIPs))
-		for i, ipBlock := range v3res.Spec.ServiceLoadBalancerIPs {
-			ipCidrs[i] = ipBlock.CIDR
+		ipCidrs := make([]string, 0, len(v3res.Spec.ServiceLoadBalancerIPs))
+		for _, ipBlock := range v3res.Spec.ServiceLoadBalancerIPs {
+			if ipBlock.CIDR == "" {
+				// The CRD allows CIDR to be optional so we just ignore empty CIDRs.
+				continue
+			}
+			ipCidrs = append(ipCidrs, ipBlock.CIDR)
 		}
 		c.updateCache(api.UpdateTypeKVUpdated, getKVPair(svcLoadBalancerIPKey, strings.Join(ipCidrs, ",")))
 	} else {
@@ -1304,9 +1312,13 @@ func (c *client) getServiceClusterIPsKVPair(v3res *apiv3.BGPConfiguration, key i
 		if v3res != nil && v3res.Spec.ServiceClusterIPs != nil && len(v3res.Spec.ServiceClusterIPs) != 0 {
 			// We wrap each Service Cluster IP in a ServiceClusterIPBlock to
 			// achieve the desired API structure. This unpacks that.
-			ipCidrs := make([]string, len(v3res.Spec.ServiceClusterIPs))
-			for i, ipBlock := range v3res.Spec.ServiceClusterIPs {
-				ipCidrs[i] = ipBlock.CIDR
+			ipCidrs := make([]string, 0, len(v3res.Spec.ServiceClusterIPs))
+			for _, ipBlock := range v3res.Spec.ServiceClusterIPs {
+				if ipBlock.CIDR == "" {
+					// The CRD allows CIDR to be optional so we just ignore empty CIDRs.
+					continue
+				}
+				ipCidrs = append(ipCidrs, ipBlock.CIDR)
 			}
 			c.updateCache(api.UpdateTypeKVUpdated, getKVPair(svcInternalIPKey, strings.Join(ipCidrs, ",")))
 		} else {

diff --git a/confd/tests/mock_data/calicoctl/mesh/static-routes/input.yaml b/confd/tests/mock_data/calicoctl/mesh/static-routes/input.yaml
@@ -5,9 +5,11 @@ metadata:
 spec:
   serviceClusterIPs:
   - cidr: 10.101.0.0/16
+  - {}
   - cidr: fd00:96::/112
   serviceLoadBalancerIPs:
   - cidr: 80.15.0.0/24
+  - {}
 
 ---
 

diff --git a/felix/calc/event_sequencer.go b/felix/calc/event_sequencer.go
@@ -968,12 +968,27 @@ func (buf *EventSequencer) OnGlobalBGPConfigUpdate(cfg *v3.BGPConfiguration) {
 	buf.pendingGlobalBGPConfig = &proto.GlobalBGPConfigUpdate{}
 	if cfg != nil {
 		for _, block := range cfg.Spec.ServiceClusterIPs {
+			if block.CIDR == "" {
+				// When we defined the CRD we allowed this field to be optional
+				// for extensibility, ignore empty CIDRs.
+				continue
+			}
 			buf.pendingGlobalBGPConfig.ServiceClusterCidrs = append(buf.pendingGlobalBGPConfig.ServiceClusterCidrs, block.CIDR)
 		}
 		for _, block := range cfg.Spec.ServiceExternalIPs {
+			if block.CIDR == "" {
+				// When we defined the CRD we allowed this field to be optional
+				// for extensibility, ignore empty CIDRs.
+				continue
+			}
 			buf.pendingGlobalBGPConfig.ServiceExternalCidrs = append(buf.pendingGlobalBGPConfig.ServiceExternalCidrs, block.CIDR)
 		}
 		for _, block := range cfg.Spec.ServiceLoadBalancerIPs {
+			if block.CIDR == "" {
+				// When we defined the CRD we allowed this field to be optional
+				// for extensibility, ignore empty CIDRs.
+				continue
+			}
 			buf.pendingGlobalBGPConfig.ServiceLoadbalancerCidrs = append(buf.pendingGlobalBGPConfig.ServiceLoadbalancerCidrs, block.CIDR)
 		}
 	}