bgpv1: Add unit tests for BGP CRD types

Add unit tests to newly added SetDefaults() and Validate()
methods of the BGP CRD types. Do not perform detailed
defaulting / validation checks in unit tests of other packages,
to have that testing logic co-located within a single package.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>

pkg/bgpv1/agent/controller_test.go

@@ -140,18 +140,12 @@ func TestControllerSanity(t *testing.T) {
 				for _, r := range p.Spec.VirtualRouters {
 					for _, n := range r.Neighbors {
 						if n.PeerPort == nil ||
+							n.EBGPMultihopTTL == nil ||
 							n.ConnectRetryTimeSeconds == nil ||
 							n.HoldTimeSeconds == nil ||
 							n.KeepAliveTimeSeconds == nil ||
 							n.GracefulRestart.RestartTimeSeconds == nil {
-							t.Fatalf("policy: %v not defaulted properly (nil)", p)
-						}
-						if *n.PeerPort != v2alpha1api.DefaultBGPPeerPort ||
-							*n.ConnectRetryTimeSeconds != v2alpha1api.DefaultBGPConnectRetryTimeSeconds ||
-							*n.HoldTimeSeconds != v2alpha1api.DefaultBGPHoldTimeSeconds ||
-							*n.KeepAliveTimeSeconds != v2alpha1api.DefaultBGPKeepAliveTimeSeconds ||
-							*n.GracefulRestart.RestartTimeSeconds != v2alpha1api.DefaultBGPGRRestartTimeSeconds {
-							t.Fatalf("policy: %v not defaulted properly (invalid value)", p)
+							t.Fatalf("policy: %v not defaulted properly", p)
 						}
 					}
 				}

pkg/k8s/apis/cilium.io/v2alpha1/bgpp_types_test.go

@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Authors of Cilium
+
+package v2alpha1
+
+import (
+	"fmt"
+	"testing"
+
+	"k8s.io/utils/pointer"
+)
+
+func TestBGPPeeringPolicyDefaulting(t *testing.T) {
+	var testPolicy = &CiliumBGPPeeringPolicy{
+		Spec: CiliumBGPPeeringPolicySpec{
+			VirtualRouters: []CiliumBGPVirtualRouter{{LocalASN: 65000}},
+		},
+	}
+	var steps = []struct {
+		description      string
+		neighbors        []CiliumBGPNeighbor
+		validateDefaults func(p *CiliumBGPPeeringPolicy) bool
+	}{
+		{
+			description: "simple policy defaulting",
+			neighbors: []CiliumBGPNeighbor{
+				{
+					PeerASN:     65001,
+					PeerAddress: "172.0.0.1/32",
+				},
+				{
+					PeerASN:     65002,
+					PeerAddress: "172.0.0.2/32",
+				},
+			},
+			validateDefaults: func(p *CiliumBGPPeeringPolicy) bool {
+				for _, r := range p.Spec.VirtualRouters {
+					if *r.ExportPodCIDR != DefaultBGPExportPodCIDR {
+						return false
+					}
+					for _, n := range r.Neighbors {
+						if *n.PeerPort != DefaultBGPPeerPort ||
+							*n.EBGPMultihopTTL != DefaultBGPEBGPMultihopTTL ||
+							*n.ConnectRetryTimeSeconds != DefaultBGPConnectRetryTimeSeconds ||
+							*n.HoldTimeSeconds != DefaultBGPHoldTimeSeconds ||
+							*n.KeepAliveTimeSeconds != DefaultBGPKeepAliveTimeSeconds {
+							return false
+						}
+					}
+				}
+				return true
+			},
+		},
+		{
+			description: "graceful restart defaulting",
+			neighbors: []CiliumBGPNeighbor{
+				{
+					PeerASN:     65001,
+					PeerAddress: "172.0.0.1/32",
+					GracefulRestart: &CiliumBGPNeighborGracefulRestart{
+						Enabled: true,
+					},
+				},
+			},
+			validateDefaults: func(p *CiliumBGPPeeringPolicy) bool {
+				for _, r := range p.Spec.VirtualRouters {
+					for _, n := range r.Neighbors {
+						if *n.GracefulRestart.RestartTimeSeconds != DefaultBGPGRRestartTimeSeconds {
+							return false
+						}
+					}
+				}
+				return true
+			},
+		},
+	}
+	for _, step := range steps {
+		t.Run(step.description, func(t *testing.T) {
+			p := testPolicy.DeepCopy()
+			p.Spec.VirtualRouters[0].Neighbors = step.neighbors
+
+			p.SetDefaults()
+			if !step.validateDefaults(p) {
+				t.Fatalf("policy: not defaulted properly")
+			}
+		})
+	}
+}
+
+func TestBGPNeighborValidation(t *testing.T) {
+	var steps = []struct {
+		description string
+		neighbor    *CiliumBGPNeighbor
+		expectError error
+	}{
+		{
+			description: "empty timers",
+			neighbor: &CiliumBGPNeighbor{
+				PeerASN:     65001,
+				PeerAddress: "172.0.0.1/32",
+			},
+			expectError: nil,
+		},
+		{
+			description: "correct timers",
+			neighbor: &CiliumBGPNeighbor{
+				PeerASN:              65001,
+				PeerAddress:          "172.0.0.1/32",
+				KeepAliveTimeSeconds: pointer.Int32(3),
+				HoldTimeSeconds:      pointer.Int32(9),
+			},
+			expectError: nil,
+		},
+		{
+			description: "incorrect timers",
+			neighbor: &CiliumBGPNeighbor{
+				PeerASN:     65001,
+				PeerAddress: "172.0.0.1/32",
+				// KeepAliveTimeSeconds larger than HoldTimeSeconds = error
+				KeepAliveTimeSeconds: pointer.Int32(10),
+				HoldTimeSeconds:      pointer.Int32(5),
+			},
+			expectError: fmt.Errorf("some-error"),
+		},
+		{
+			description: "incorrect timers with default value",
+			neighbor: &CiliumBGPNeighbor{
+				PeerASN:     65001,
+				PeerAddress: "172.0.0.1/32",
+				// KeepAliveTimeSeconds larger than default HoldTimeSeconds (90) = error
+				KeepAliveTimeSeconds: pointer.Int32(100),
+			},
+			expectError: fmt.Errorf("some-error"),
+		},
+	}
+	for _, step := range steps {
+		t.Run(step.description, func(t *testing.T) {
+			err := step.neighbor.Validate()
+			if (step.expectError == nil) != (err == nil) {
+				t.Fatalf("incorrect validation result - want: %v, got: %v", step.expectError, err)
+			}
+		})
+	}
+}