Add confd integration tests for BGPFilter

confd/pkg/resource/template/template_funcs.go

@@ -381,145 +381,105 @@ func hasPeerTypeRules(rules []filterArgs) bool {
 	return false
 }
 
-// emitFilterRules generates the BIRD filter function body lines for the given rules.
-// Rules with PeerType are wrapped in if (is_same_as) / if (!is_same_as) guards.
-func emitFilterRules(ruleFields []filterArgs) ([]string, error) {
-	var lines []string
-	for _, fields := range ruleFields {
-		filterRule, err := filterStatement(fields)
-		if err != nil {
-			return nil, err
-		}
-
-		// Wrap the rule in a PeerType guard if the rule specifies a PeerType.
-		switch fields.peerType {
-		case v3.BGPFilterPeerTypeIBGP:
-			filterRule = fmt.Sprintf("if (is_same_as) then { %s }", filterRule)
-		case v3.BGPFilterPeerTypeEBGP:
-			filterRule = fmt.Sprintf("if (!is_same_as) then { %s }", filterRule)
-		}
-
-		lines = append(lines, fmt.Sprintf("  %s", filterRule))
-	}
-	return lines, nil
-}
-
-// BGPFilterBIRDFuncs generates a set of BIRD functions for BGPFilter resources that have been packaged into KVPairs.
-// By doing the formatting inside of this function we eliminate the need to copy and paste repeated blocks of golang
-// template code into our BIRD config templates that is both difficult to read and prone to errors.
+// BGPFilterBIRDFuncs generates the definitions of a set of BIRD functions for all configured BGPFilter resources.
 //
-// When any rule within a filter uses PeerType, the generated function takes a bool parameter:
+// For each direction (import/export), if any rule for that direction within a filter uses PeerType,
+// the generated function for that direction takes a bool parameter:
 //
 //	function 'bgp_myfilter_importFilterV4'(bool is_same_as) { ... }
 //
-// Rules with PeerType are then wrapped in if (is_same_as) / if (!is_same_as) guards.
+// Within such a function, rules with PeerType are wrapped in if (is_same_as) / if (!is_same_as) guards.
 func BGPFilterBIRDFuncs(pairs memkv.KVPairs, version int) ([]string, error) {
-	lines := []string{}
-	var line string
+	var lines []string
 	var versionStr string
 
 	if version == 4 || version == 6 {
 		versionStr = fmt.Sprintf("%d", version)
 	} else {
-		return []string{}, fmt.Errorf("version must be either 4 or 6")
+		return nil, fmt.Errorf("version must be either 4 or 6")
+	}
+
+	v4Selected := version == 4
+
+	type directionInput struct {
+		dir string
+		v4  []v3.BGPFilterRuleV4
+		v6  []v3.BGPFilterRuleV6
+	}
+	type directionRules struct {
+		direction string
+		rules     []filterArgs
 	}
 
 	for _, kvp := range pairs {
 		var filter v3.BGPFilter
 		err := json.Unmarshal([]byte(kvp.Value), &filter)
 		if err != nil {
-			return []string{}, fmt.Errorf("error unmarshalling JSON: %s", err)
+			return nil, fmt.Errorf("error unmarshalling JSON: %w", err)
 		}
 
-		var filterName string
-		var emitImports bool
-		var emitExports bool
-		v4Selected := version == 4
-
-		if v4Selected {
-			emitImports = len(filter.Spec.ImportV4) > 0
-			emitExports = len(filter.Spec.ExportV4) > 0
-		} else {
-			emitImports = len(filter.Spec.ImportV6) > 0
-			emitExports = len(filter.Spec.ExportV6) > 0
-		}
-
-		if emitImports || emitExports {
-			filterName = path.Base(kvp.Key)
-			line = fmt.Sprintf("# v%s BGPFilter %s", versionStr, filterName)
-			lines = append(lines, line)
-		}
-
-		var filterFuncName string
-		if emitImports {
-			filterFuncName, err = BGPFilterFunctionName(filterName, "import", versionStr)
-			if err != nil {
-				return []string{}, err
-			}
-
+		// Build rules for each direction, converting V4/V6 to unified filterArgs.
+		var directions []directionRules
+		for _, d := range []directionInput{
+			{"import", filter.Spec.ImportV4, filter.Spec.ImportV6},
+			{"export", filter.Spec.ExportV4, filter.Spec.ExportV6},
+		} {
 			var ruleFields []filterArgs
 			if v4Selected {
-				for _, rule := range filter.Spec.ImportV4 {
+				for _, rule := range d.v4 {
 					ruleFields = append(ruleFields, filterArgsFromRuleV4(rule))
 				}
 			} else {
-				for _, rule := range filter.Spec.ImportV6 {
+				for _, rule := range d.v6 {
 					ruleFields = append(ruleFields, filterArgsFromRuleV6(rule))
 				}
 			}
-
-			if hasPeerTypeRules(ruleFields) {
-				line = fmt.Sprintf("function %s(bool is_same_as) {", filterFuncName)
-			} else {
-				line = fmt.Sprintf("function %s() {", filterFuncName)
-			}
-			lines = append(lines, line)
-
-			ruleLines, err := emitFilterRules(ruleFields)
-			if err != nil {
-				return []string{}, err
+			if len(ruleFields) > 0 {
+				directions = append(directions, directionRules{d.dir, ruleFields})
 			}
-			lines = append(lines, ruleLines...)
+		}
 
-			lines = append(lines, "}")
+		if len(directions) == 0 {
+			continue
 		}
 
-		if emitExports {
-			filterFuncName, err = BGPFilterFunctionName(filterName, "export", versionStr)
+		filterName := path.Base(kvp.Key)
+		lines = append(lines, fmt.Sprintf("# v%s BGPFilter %s", versionStr, filterName))
+
+		for _, dr := range directions {
+			filterFuncName, err := BGPFilterFunctionName(filterName, dr.direction, versionStr)
 			if err != nil {
-				return []string{}, err
+				return nil, err
 			}
 
-			var ruleFields []filterArgs
-			if v4Selected {
-				for _, rule := range filter.Spec.ExportV4 {
-					ruleFields = append(ruleFields, filterArgsFromRuleV4(rule))
-				}
+			if hasPeerTypeRules(dr.rules) {
+				lines = append(lines, fmt.Sprintf("function %s(bool is_same_as) {", filterFuncName))
 			} else {
-				for _, rule := range filter.Spec.ExportV6 {
-					ruleFields = append(ruleFields, filterArgsFromRuleV6(rule))
-				}
+				lines = append(lines, fmt.Sprintf("function %s() {", filterFuncName))
 			}
 
-			if hasPeerTypeRules(ruleFields) {
-				line = fmt.Sprintf("function %s(bool is_same_as) {", filterFuncName)
-			} else {
-				line = fmt.Sprintf("function %s() {", filterFuncName)
-			}
-			lines = append(lines, line)
+			// Emit each rule as a BIRD statement, wrapping with PeerType guard if needed.
+			for _, fields := range dr.rules {
+				filterRule, err := filterStatement(fields)
+				if err != nil {
+					return nil, err
+				}
 
-			ruleLines, err := emitFilterRules(ruleFields)
-			if err != nil {
-				return []string{}, err
+				switch fields.peerType {
+				case v3.BGPFilterPeerTypeIBGP:
+					filterRule = fmt.Sprintf("if (is_same_as) then { %s }", filterRule)
+				case v3.BGPFilterPeerTypeEBGP:
+					filterRule = fmt.Sprintf("if (!is_same_as) then { %s }", filterRule)
+				}
+
+				lines = append(lines, fmt.Sprintf("  %s", filterRule))
 			}
-			lines = append(lines, ruleLines...)
 
 			lines = append(lines, "}")
 		}
 	}
 	if len(lines) == 0 {
-		line = fmt.Sprintf("# No v%s BGPFilters configured", versionStr)
-		lines = append(lines, line)
+		lines = append(lines, fmt.Sprintf("# No v%s BGPFilters configured", versionStr))
 	}
 	return lines, nil
 }

confd/tests/compiled_templates/bgpfilter/as_path_and_priority/bird.cfg

@@ -0,0 +1,120 @@
+function apply_communities ()
+{
+}
+
+# Generated by confd
+include "bird_aggr.cfg";
+include "bird_ipam.cfg";
+
+router id 10.192.0.2;
+
+# Configure synchronization between routing tables and kernel.
+protocol kernel {
+  learn;             # Learn all alien routes from the kernel
+  persist;           # Don't remove routes on bird shutdown
+  scan time 2;       # Scan kernel routing table every 2 seconds
+  import all;
+  export filter calico_kernel_programming; # Default is export none
+  graceful restart;  # Turn on graceful restart to reduce potential flaps in
+                     # routes when reloading BIRD configuration.  With a full
+                     # automatic mesh, there is no way to prevent BGP from
+                     # flapping since multiple nodes update their BGP
+                     # configuration at the same time, GR is not guaranteed to
+                     # work correctly in this scenario.
+  merge paths on;    # Allow export multipath routes (ECMP)
+}
+
+# Watch interface up/down events.
+protocol device {
+  debug { states };
+  scan time 2;    # Scan interfaces every 2 seconds
+}
+
+protocol direct {
+  debug { states };
+  interface -"cali*", -"kube-ipvs*", "*"; # Exclude cali* and kube-ipvs* but
+                                          # include everything else.  In
+                                          # IPVS-mode, kube-proxy creates a
+                                          # kube-ipvs0 interface. We exclude
+                                          # kube-ipvs0 because this interface
+                                          # gets an address for every in use
+                                          # cluster IP. We use static routes
+                                          # for when we legitimately want to
+                                          # export cluster IPs.
+}
+
+
+# Template for all BGP clients
+template bgp bgp_template {
+  debug { states };
+  description "Connection to BGP peer";
+  local as 64512;
+  gateway recursive; # This should be the default, but just in case.
+  add paths on;
+  graceful restart;  # See comment in kernel section about graceful restart.
+  connect delay time 2;
+  connect retry time 5;
+  error wait time 5,30;
+}
+
+# -------------- BGP Filters ------------------
+# v4 BGPFilter test-filter-aspath-prio
+function 'bgp_test-filter-aspath-prio_importFilterV4'() {
+  if ((bgp_path ~ [= 65000 * =])) then { krt_metric = 100; accept; }
+  if ((bgp_path ~ [= 65000 65001 * =])) then { krt_metric = 200; accept; }
+  accept;
+}
+function 'bgp_test-filter-aspath-prio_exportFilterV4'() {
+  if ((krt_metric = 100)) then { bgp_community.add((65000, 100)); accept; }
+  if ((krt_metric = 200)) then { bgp_path.prepend(65001); bgp_path.prepend(65000); accept; }
+  reject;
+}
+# BGP Protocol Configurations
+protocol bgp Global_10_192_0_3 from bgp_template {
+  ttl security off;
+  multihop;
+  neighbor 10.192.0.3 as 64512;
+  source address 10.192.0.2;  # The local address we use for the TCP connection
+  import filter {
+    krt_metric = 1024;
+    if (defined(bgp_local_pref)) then {
+      krt_metric = 2147483647 - bgp_local_pref;
+    }
+    'bgp_test-filter-aspath-prio_importFilterV4'();
+    if (krt_metric < 1024) then
+      preference = 200;
+    accept;
+  };
+  export filter {
+    if (!defined(krt_metric)) then { krt_metric = 1024; }
+    bgp_local_pref = 2147483647 - krt_metric;
+    'bgp_test-filter-aspath-prio_exportFilterV4'();
+    calico_export_to_bgp_peers(true);
+    reject;
+  };  # Only want to export routes for workloads.
+}
+protocol bgp Global_10_192_0_4 from bgp_template {
+  ttl security off;
+  multihop;
+  neighbor 10.192.0.4 as 64512;
+  source address 10.192.0.2;  # The local address we use for the TCP connection
+  import filter {
+    krt_metric = 1024;
+    if (defined(bgp_local_pref)) then {
+      krt_metric = 2147483647 - bgp_local_pref;
+    }
+    'bgp_test-filter-aspath-prio_importFilterV4'();
+    if (krt_metric < 1024) then
+      preference = 200;
+    accept;
+  };
+  export filter {
+    if (!defined(krt_metric)) then { krt_metric = 1024; }
+    bgp_local_pref = 2147483647 - krt_metric;
+    'bgp_test-filter-aspath-prio_exportFilterV4'();
+    calico_export_to_bgp_peers(true);
+    reject;
+  };  # Only want to export routes for workloads.
+}
+
+