From 9e8a14ef3d654aef2cc5b7965471901d3a1ba812 Mon Sep 17 00:00:00 2001
From: SkalaNetworks <contact@skala.network>
Date: Sun, 21 Jul 2024 12:40:54 -0400
Subject: [PATCH] feat(natgw): throw error when nad undefined

Signed-off-by: SkalaNetworks <contact@skala.network>
---
 pkg/controller/controller.go      |  2 +-
 pkg/controller/vpc_nat.go         |  8 +++++++-
 pkg/controller/vpc_nat_gateway.go | 24 ++++++++++++++++++------
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
index e3bafdef480f..e5875b317309 100644
--- a/pkg/controller/controller.go
+++ b/pkg/controller/controller.go
@@ -1032,7 +1032,7 @@ func (c *Controller) startWorkers(ctx context.Context) {
 	}, time.Second, ctx.Done())
 
 	go wait.Until(func() {
-		c.resyncVpcNatImage()
+		c.resyncVpcNatConfig()
 	}, time.Second, ctx.Done())
 
 	go wait.Until(func() {
diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go
index 16ce0ee477e1..b85fa5091d8c 100644
--- a/pkg/controller/vpc_nat.go
+++ b/pkg/controller/vpc_nat.go
@@ -11,9 +11,10 @@ import (
 var (
 	vpcNatImage             = ""
 	vpcNatGwBgpSpeakerImage = ""
+	vpcNatApiNadProvider    = ""
 )
 
-func (c *Controller) resyncVpcNatImage() {
+func (c *Controller) resyncVpcNatConfig() {
 	cm, err := c.configMapsLister.ConfigMaps(c.config.PodNamespace).Get(util.VpcNatConfig)
 	if err != nil {
 		err = fmt.Errorf("failed to get ovn-vpc-nat-config, %v", err)
@@ -21,6 +22,7 @@ func (c *Controller) resyncVpcNatImage() {
 		return
 	}
 
+	// Image we're using to provision the NAT gateways
 	image, exist := cm.Data["image"]
 	if !exist {
 		err = fmt.Errorf("%s should have image field", util.VpcNatConfig)
@@ -29,5 +31,9 @@ func (c *Controller) resyncVpcNatImage() {
 	}
 	vpcNatImage = image
 
+	// Image for the BGP sidecar of the gateway (optional)
 	vpcNatGwBgpSpeakerImage = cm.Data["bgpSpeakerImage"]
+
+	// NetworkAttachmentDefinition for the BGP speaker to call the API server
+	vpcNatApiNadProvider = cm.Data["apiNadProvider"]
 }
diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go
index 7586dbbafd53..b114c585ac62 100644
--- a/pkg/controller/vpc_nat_gateway.go
+++ b/pkg/controller/vpc_nat_gateway.go
@@ -736,14 +736,14 @@ func (c *Controller) execNatGwRules(pod *corev1.Pod, operation string, rules []s
 	return nil
 }
 
-func (c *Controller) setNatGwInterface(annotations map[string]string, externalNetwork string, defaultSubnet *kubeovnv1.Subnet) {
+func (c *Controller) setNatGwInterface(annotations map[string]string, externalNetwork string, defaultSubnet *kubeovnv1.Subnet) error {
 	nad := fmt.Sprintf("%s/%s, %s/%s", c.config.PodNamespace, externalNetwork, corev1.NamespaceDefault, nadName)
 	annotations[util.AttachmentNetworkAnnotation] = nad
 
-	setNatGwRoute(annotations, defaultSubnet.Spec.Gateway)
+	return setNatGwRoute(annotations, defaultSubnet.Spec.Gateway)
 }
 
-func setNatGwRoute(annotations map[string]string, subnetGw string) {
+func setNatGwRoute(annotations map[string]string, subnetGw string) error {
 	dst := os.Getenv("KUBERNETES_SERVICE_HOST")
 
 	protocol := util.CheckProtocol(dst)
@@ -755,18 +755,27 @@ func setNatGwRoute(annotations map[string]string, subnetGw string) {
 			dst = fmt.Sprintf("%s/128", dst)
 		}
 	}
+
+	// Check the API NetworkAttachmentDefinition exists, otherwise we won't be able to attach
+	// the BGP speaker to a network that has access to the K8S apiserver (and won't be able to detect EIPs)
+	if vpcNatApiNadProvider == "" {
+		return fmt.Errorf("no NetworkAttachmentDefinition provided to access apiserver, check configmap ovn-vpc-nat-config and field 'apiNadProvider'")
+	}
+
 	for _, gw := range strings.Split(subnetGw, ",") {
 		if util.CheckProtocol(gw) == protocol {
 			routes := []request.Route{{Destination: dst, Gateway: gw}}
 			buf, err := json.Marshal(routes)
 			if err != nil {
-				klog.Errorf("failed to marshal routes %+v: %v", routes, err)
+				return fmt.Errorf("failed to marshal routes %+v: %v", routes, err)
 			} else {
-				annotations[fmt.Sprintf(util.RoutesAnnotationTemplate, nadProvider)] = string(buf)
+				annotations[fmt.Sprintf(util.RoutesAnnotationTemplate, vpcNatApiNadProvider)] = string(buf)
 			}
 			break
 		}
 	}
+
+	return nil
 }
 
 func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1.StatefulSet) (*v1.StatefulSet, error) {
@@ -787,7 +796,10 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1
 		if err != nil {
 			return nil, fmt.Errorf("failed to get default subnet %s: %v", c.config.DefaultLogicalSwitch, err)
 		}
-		c.setNatGwInterface(podAnnotations, nadName, defaultSubnet)
+
+		if err := c.setNatGwInterface(podAnnotations, nadName, defaultSubnet); err != nil {
+			return nil, err
+		}
 	}
 
 	for key, value := range podAnnotations {