From 9698ffc480e95da7cc5f715dfb1c950386ce1eb5 Mon Sep 17 00:00:00 2001 From: Zespre Chang Date: Fri, 2 Feb 2024 17:56:46 +0800 Subject: [PATCH] fix(webhook): validate pool-related inputs Signed-off-by: Zespre Chang --- pkg/controller/ippool/common.go | 14 +- pkg/controller/ippool/controller.go | 5 +- pkg/controller/ippool/controller_test.go | 6 +- pkg/util/network.go | 62 +++- pkg/webhook/ippool/validator.go | 129 ++++++-- pkg/webhook/ippool/validator_test.go | 372 +++++++++++++++++++++-- 6 files changed, 533 insertions(+), 55 deletions(-) diff --git a/pkg/controller/ippool/common.go b/pkg/controller/ippool/common.go index d4acae0..5c0c8a3 100644 --- a/pkg/controller/ippool/common.go +++ b/pkg/controller/ippool/common.go @@ -24,7 +24,7 @@ func prepareAgentPod( clusterNetwork string, agentServiceAccountName string, agentImage *config.Image, -) *corev1.Pod { +) (*corev1.Pod, error) { name := fmt.Sprintf("%s-%s-agent", ipPool.Namespace, ipPool.Name) nadNamespace, nadName := kv.RSplit(ipPool.Spec.NetworkName, "/") @@ -35,9 +35,15 @@ func prepareAgentPod( InterfaceName: "eth1", }, } - networksStr, _ := json.Marshal(networks) + networksStr, err := json.Marshal(networks) + if err != nil { + return nil, err + } - _, ipNet, _ := net.ParseCIDR(ipPool.Spec.IPv4Config.CIDR) + _, ipNet, err := net.ParseCIDR(ipPool.Spec.IPv4Config.CIDR) + if err != nil { + return nil, err + } prefixLength, _ := ipNet.Mask.Size() args := []string{ @@ -141,7 +147,7 @@ func prepareAgentPod( }, }, }, - } + }, nil } func setRegisteredCondition(ipPool *networkv1.IPPool, status corev1.ConditionStatus, reason, message string) { diff --git a/pkg/controller/ippool/controller.go b/pkg/controller/ippool/controller.go index 6fc4a40..1a21a57 100644 --- a/pkg/controller/ippool/controller.go +++ b/pkg/controller/ippool/controller.go @@ -280,7 +280,10 @@ func (h *Handler) DeployAgent(ipPool *networkv1.IPPool, status networkv1.IPPoolS return status, fmt.Errorf("could not find clusternetwork for nad %s", ipPool.Spec.NetworkName) } - agent := prepareAgentPod(ipPool, h.noDHCP, h.agentNamespace, clusterNetwork, h.agentServiceAccountName, h.agentImage) + agent, err := prepareAgentPod(ipPool, h.noDHCP, h.agentNamespace, clusterNetwork, h.agentServiceAccountName, h.agentImage) + if err != nil { + return status, err + } agentPod, err := h.podClient.Create(agent) if err != nil { diff --git a/pkg/controller/ippool/controller_test.go b/pkg/controller/ippool/controller_test.go index ce47c9c..3076445 100644 --- a/pkg/controller/ippool/controller_test.go +++ b/pkg/controller/ippool/controller_test.go @@ -226,7 +226,7 @@ func TestHandler_DeployAgent(t *testing.T) { expectedStatus := newTestIPPoolStatusBuilder(). AgentPodRef(testPodNamespace, testPodName).Build() - expectedPod := prepareAgentPod( + expectedPod, _ := prepareAgentPod( NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName). ServerIP(testServerIP). CIDR(testCIDR). @@ -322,7 +322,7 @@ func TestHandler_DeployAgent(t *testing.T) { AgentPodRef(testPodNamespace, testPodName).Build() givenNAD := newTestNetworkAttachmentDefinitionBuilder(). Label(clusterNetworkLabelKey, testClusterNetwork).Build() - givenPod := prepareAgentPod( + givenPod, _ := prepareAgentPod( NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName). ServerIP(testServerIP). CIDR(testCIDR). @@ -339,7 +339,7 @@ func TestHandler_DeployAgent(t *testing.T) { expectedStatus := newTestIPPoolStatusBuilder(). AgentPodRef(testPodNamespace, testPodName).Build() - expectedPod := prepareAgentPod( + expectedPod, _ := prepareAgentPod( NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName). ServerIP(testServerIP). CIDR(testCIDR). diff --git a/pkg/util/network.go b/pkg/util/network.go index 2a3bb9f..8115fee 100644 --- a/pkg/util/network.go +++ b/pkg/util/network.go @@ -4,9 +4,21 @@ import ( "fmt" "net" "net/netip" + + networkv1 "github.com/harvester/vm-dhcp-controller/pkg/apis/network.harvesterhci.io/v1alpha1" ) -func LoadCIDR(cidr string) (ipNet *net.IPNet, networkIPAddr netip.Addr, broadcastIPAddr netip.Addr, err error) { +type PoolInfo struct { + IPNet *net.IPNet + NetworkIPAddr netip.Addr + BroadcastIPAddr netip.Addr + StartIPAddr netip.Addr + EndIPAddr netip.Addr + ServerIPAddr netip.Addr + RouterIPAddr netip.Addr +} + +func loadCIDR(cidr string) (ipNet *net.IPNet, networkIPAddr netip.Addr, broadcastIPAddr netip.Addr, err error) { _, ipNet, err = net.ParseCIDR(cidr) if err != nil { return @@ -31,3 +43,51 @@ func LoadCIDR(cidr string) (ipNet *net.IPNet, networkIPAddr netip.Addr, broadcas return } + +func LoadPool(ipPool *networkv1.IPPool) (pi PoolInfo, err error) { + pi.IPNet, pi.NetworkIPAddr, pi.BroadcastIPAddr, err = loadCIDR(ipPool.Spec.IPv4Config.CIDR) + if err != nil { + return + } + + if ipPool.Spec.IPv4Config.Pool.Start != "" { + pi.StartIPAddr, err = netip.ParseAddr(ipPool.Spec.IPv4Config.Pool.Start) + if err != nil { + return + } + } + + if ipPool.Spec.IPv4Config.Pool.End != "" { + pi.EndIPAddr, err = netip.ParseAddr(ipPool.Spec.IPv4Config.Pool.End) + if err != nil { + return + } + } + + if ipPool.Spec.IPv4Config.ServerIP != "" { + pi.ServerIPAddr, err = netip.ParseAddr(ipPool.Spec.IPv4Config.ServerIP) + if err != nil { + return + } + } + + if ipPool.Spec.IPv4Config.Router != "" { + pi.RouterIPAddr, err = netip.ParseAddr(ipPool.Spec.IPv4Config.Router) + if err != nil { + return + } + } + + return +} + +func LoadAllocated(allocated map[string]string) (ipAddrList []netip.Addr) { + for ip := range allocated { + ipAddr, err := netip.ParseAddr(ip) + if err != nil { + continue + } + ipAddrList = append(ipAddrList, ipAddr) + } + return +} diff --git a/pkg/webhook/ippool/validator.go b/pkg/webhook/ippool/validator.go index e6746c9..131fc9d 100644 --- a/pkg/webhook/ippool/validator.go +++ b/pkg/webhook/ippool/validator.go @@ -36,11 +36,25 @@ func (v *Validator) Create(_ *admission.Request, newObj runtime.Object) error { ipPool := newObj.(*networkv1.IPPool) logrus.Infof("create ippool %s/%s", ipPool.Namespace, ipPool.Name) - if err := v.checkNAD(ipPool); err != nil { + // sanity check + poolInfo, err := util.LoadPool(ipPool) + if err != nil { + return fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) + } + + if err := v.checkNAD(ipPool.Spec.NetworkName); err != nil { + return fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) + } + + if err := v.checkPoolRange(poolInfo); err != nil { return fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) } - if err := v.checkServerIP(ipPool); err != nil { + if err := v.checkServerIP(poolInfo); err != nil { + return fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) + } + + if err := v.checkRouter(poolInfo); err != nil { return fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) } @@ -56,11 +70,30 @@ func (v *Validator) Update(_ *admission.Request, _, newObj runtime.Object) error logrus.Infof("update ippool %s/%s", ipPool.Namespace, ipPool.Name) - if err := v.checkNAD(ipPool); err != nil { + // sanity check + poolInfo, err := util.LoadPool(ipPool) + if err != nil { return fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) } - if err := v.checkServerIP(ipPool); err != nil { + var allocatedIPAddrList []netip.Addr + if ipPool.Status.IPv4 != nil { + allocatedIPAddrList = util.LoadAllocated(ipPool.Status.IPv4.Allocated) + } + + if err := v.checkNAD(ipPool.Spec.NetworkName); err != nil { + return fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) + } + + if err := v.checkPoolRange(poolInfo); err != nil { + return fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) + } + + if err := v.checkServerIP(poolInfo, allocatedIPAddrList...); err != nil { + return fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) + } + + if err := v.checkRouter(poolInfo); err != nil { return fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) } @@ -93,8 +126,8 @@ func (v *Validator) Resource() admission.Resource { } } -func (v *Validator) checkNAD(ipPool *networkv1.IPPool) error { - nadNamespace, nadName := kv.RSplit(ipPool.Spec.NetworkName, "/") +func (v *Validator) checkNAD(namespacedName string) error { + nadNamespace, nadName := kv.RSplit(namespacedName, "/") if nadNamespace == "" { nadNamespace = "default" } @@ -103,49 +136,87 @@ func (v *Validator) checkNAD(ipPool *networkv1.IPPool) error { return err } -func (v *Validator) checkServerIP(ipPool *networkv1.IPPool) error { - ipNet, networkIPAddr, broadcastIPAddr, err := util.LoadCIDR(ipPool.Spec.IPv4Config.CIDR) - if err != nil { - return err +func (v *Validator) checkPoolRange(pi util.PoolInfo) error { + if pi.StartIPAddr.IsValid() { + if !pi.IPNet.Contains(pi.StartIPAddr.AsSlice()) { + return fmt.Errorf("start ip %s is not within subnet", pi.StartIPAddr) + } + + if pi.StartIPAddr.As4() == pi.NetworkIPAddr.As4() { + return fmt.Errorf("start ip %s is the same as network ip", pi.StartIPAddr) + } + + if pi.StartIPAddr.As4() == pi.BroadcastIPAddr.As4() { + return fmt.Errorf("start ip %s is the same as broadcast ip", pi.StartIPAddr) + } } - routerIPAddr, err := netip.ParseAddr(ipPool.Spec.IPv4Config.Router) - if err != nil { - routerIPAddr = netip.Addr{} + if pi.EndIPAddr.IsValid() { + if !pi.IPNet.Contains(pi.EndIPAddr.AsSlice()) { + return fmt.Errorf("end ip %s is not within subnet", pi.EndIPAddr) + } + + if pi.EndIPAddr.As4() == pi.NetworkIPAddr.As4() { + return fmt.Errorf("end ip %s is the same as network ip", pi.EndIPAddr) + } + + if pi.EndIPAddr.As4() == pi.BroadcastIPAddr.As4() { + return fmt.Errorf("end ip %s is the same as broadcast ip", pi.EndIPAddr) + } } + return nil +} - serverIPAddr, err := netip.ParseAddr(ipPool.Spec.IPv4Config.ServerIP) - if err != nil { - return err +func (v *Validator) checkServerIP(pi util.PoolInfo, allocatedIPs ...netip.Addr) error { + if !pi.ServerIPAddr.IsValid() { + return nil } - if !ipNet.Contains(serverIPAddr.AsSlice()) { - return fmt.Errorf("server ip %s is not within subnet", serverIPAddr) + if !pi.IPNet.Contains(pi.ServerIPAddr.AsSlice()) { + return fmt.Errorf("server ip %s is not within subnet", pi.ServerIPAddr) } - if serverIPAddr.As4() == networkIPAddr.As4() { - return fmt.Errorf("server ip %s cannot be the same as network ip", serverIPAddr) + if pi.ServerIPAddr.As4() == pi.NetworkIPAddr.As4() { + return fmt.Errorf("server ip %s cannot be the same as network ip", pi.ServerIPAddr) } - if serverIPAddr.As4() == broadcastIPAddr.As4() { - return fmt.Errorf("server ip %s cannot be the same as broadcast ip", serverIPAddr) + if pi.ServerIPAddr.As4() == pi.BroadcastIPAddr.As4() { + return fmt.Errorf("server ip %s cannot be the same as broadcast ip", pi.ServerIPAddr) } - if routerIPAddr.IsValid() && serverIPAddr.As4() == routerIPAddr.As4() { - return fmt.Errorf("server ip %s cannot be the same as router ip", serverIPAddr) + if pi.RouterIPAddr.IsValid() && pi.ServerIPAddr.As4() == pi.RouterIPAddr.As4() { + return fmt.Errorf("server ip %s cannot be the same as router ip", pi.ServerIPAddr) } - if ipPool.Status.IPv4 != nil { - for ip, mac := range ipPool.Status.IPv4.Allocated { - if serverIPAddr.String() == ip { - return fmt.Errorf("server ip %s is already allocated by mac %s", serverIPAddr, mac) - } + for _, ip := range allocatedIPs { + if pi.ServerIPAddr == ip { + return fmt.Errorf("server ip %s is already allocated", pi.ServerIPAddr) } } return nil } +func (v *Validator) checkRouter(pi util.PoolInfo) error { + if !pi.RouterIPAddr.IsValid() { + return nil + } + + if !pi.IPNet.Contains(pi.RouterIPAddr.AsSlice()) { + return fmt.Errorf("router ip %s is not within subnet", pi.RouterIPAddr) + } + + if pi.RouterIPAddr.As4() == pi.NetworkIPAddr.As4() { + return fmt.Errorf("router ip %s is the same as network ip", pi.RouterIPAddr) + } + + if pi.RouterIPAddr.As4() == pi.BroadcastIPAddr.As4() { + return fmt.Errorf("router ip %s is the same as broadcast ip", pi.BroadcastIPAddr) + } + + return nil +} + func (v *Validator) checkVmNetCfgs(ipPool *networkv1.IPPool) error { vmnetcfgGetter := util.VmnetcfgGetter{ VmnetcfgCache: v.vmnetcfgCache, diff --git a/pkg/webhook/ippool/validator_test.go b/pkg/webhook/ippool/validator_test.go index e9e7a4a..309b294 100644 --- a/pkg/webhook/ippool/validator_test.go +++ b/pkg/webhook/ippool/validator_test.go @@ -20,6 +20,7 @@ const ( testNADName = "net-1" testIPPoolNamespace = testNADNamespace testIPPoolName = testNADName + testCIDR = "192.168.0.0/24" testServerIPOutOfRange = "192.168.100.2" testRouter = "192.168.0.1" testNetworkName = testNADNamespace + "/" + testNADName @@ -111,6 +112,174 @@ func TestValidator_Create(t *testing.T) { err: fmt.Errorf("could not create IPPool %s/%s because server ip %s cannot be the same as router ip", testIPPoolNamespace, testIPPoolName, "192.168.0.254"), }, }, + { + name: "invalid router ip which is malformed", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + Router("192.168.0.1000"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), + }, + }, + { + name: "invalid router ip which is out of subnet", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + Router("192.168.1.1"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because router ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.1"), + }, + }, + { + name: "invalid router ip which is the same as network ip", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + Router("192.168.0.0"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because router ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), + }, + }, + { + name: "invalid router ip which is the same as broadcast ip", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + Router("192.168.0.255"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because router ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), + }, + }, + { + name: "invalid start ip which is malformed", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + PoolRange("192.168.0.1000", ""). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), + }, + }, + { + name: "invalid start ip which is out of subnet", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + PoolRange("192.168.1.100", ""). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because start ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.100"), + }, + }, + { + name: "invalid start ip which is the same as network ip", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + PoolRange("192.168.0.0", ""). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because start ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), + }, + }, + { + name: "invalid start ip which is the same as broadcast ip", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + PoolRange("192.168.0.255", ""). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because start ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), + }, + }, + { + name: "invalid end ip which is malformed", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + PoolRange("", "192.168.0.1000"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), + }, + }, + { + name: "invalid end ip which is out of subnet", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + PoolRange("", "192.168.1.100"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because end ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.100"), + }, + }, + { + name: "invalid emd ip which is the same as network ip", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + PoolRange("", "192.168.0.0"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because end ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), + }, + }, + { + name: "invalid end ip which is the same as broadcast ip", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + PoolRange("", "192.168.0.255"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because end ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), + }, + }, + { + name: "non-existed network name", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + NetworkName("nonexist").Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool default/net-1 because network-attachment-definitions.k8s.cni.cncf.io \"%s\" not found", "nonexist"), + }, + }, } nadGVR := schema.GroupVersionResource{ @@ -158,11 +327,11 @@ func TestValidator_Update(t *testing.T) { name: "valid server ip", given: input{ oldIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.0/24"). + CIDR(testCIDR). ServerIP("192.168.0.2"). NetworkName(testNetworkName).Build(), newIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.0/24"). + CIDR(testCIDR). ServerIP("192.168.0.254"). NetworkName(testNetworkName).Build(), nad: newTestNetworkAttachmentDefinitionBuilder().Build(), @@ -172,11 +341,11 @@ func TestValidator_Update(t *testing.T) { name: "invalid server ip which is out of range", given: input{ oldIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.0/24"). + CIDR(testCIDR). ServerIP("192.168.0.2"). NetworkName(testNetworkName).Build(), newIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.0/24"). + CIDR(testCIDR). ServerIP("192.168.100.2"). NetworkName(testNetworkName).Build(), nad: newTestNetworkAttachmentDefinitionBuilder().Build(), @@ -189,45 +358,46 @@ func TestValidator_Update(t *testing.T) { name: "invalid server ip which is the same as network ip", given: input{ oldIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.0/24"). + CIDR(testCIDR). ServerIP("192.168.0.2"). NetworkName(testNetworkName).Build(), newIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.128/25"). - ServerIP("192.168.0.128"). + CIDR(testCIDR). + ServerIP("192.168.0.0"). NetworkName(testNetworkName).Build(), nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not create IPPool %s/%s because server ip %s cannot be the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.128"), + err: fmt.Errorf("could not create IPPool %s/%s because server ip %s cannot be the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), }, }, { name: "invalid server ip which is the same as broadcast ip", given: input{ oldIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.0/24"). + CIDR(testCIDR). ServerIP("192.168.0.2"). NetworkName(testNetworkName).Build(), newIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.0/25"). - ServerIP("192.168.0.127"). + CIDR(testCIDR). + ServerIP("192.168.0.255"). NetworkName(testNetworkName).Build(), nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not create IPPool %s/%s because server ip %s cannot be the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.127"), + err: fmt.Errorf("could not create IPPool %s/%s because server ip %s cannot be the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), }, }, { name: "invalid server ip which is the same as router ip", given: input{ oldIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.0/24"). + CIDR(testCIDR). ServerIP("192.168.0.2"). + Router("192.168.0.254"). NetworkName(testNetworkName).Build(), newIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.254/24"). + CIDR(testCIDR). ServerIP("192.168.0.254"). Router("192.168.0.254"). NetworkName(testNetworkName).Build(), @@ -242,19 +412,187 @@ func TestValidator_Update(t *testing.T) { given: input{ oldIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.254/24"). + CIDR(testCIDR). ServerIP("192.168.0.2"). NetworkName(testNetworkName). Allocated("192.168.0.100", "11:22:33:44:55:66").Build(), newIPPool: newTestIPPoolBuilder(). - CIDR("192.168.0.254/24"). + CIDR(testCIDR). ServerIP("192.168.0.100"). NetworkName(testNetworkName). Allocated("192.168.0.100", "11:22:33:44:55:66").Build(), nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not create IPPool %s/%s because server ip %s is already allocated by mac %s", testIPPoolNamespace, testIPPoolName, "192.168.0.100", "11:22:33:44:55:66"), + err: fmt.Errorf("could not create IPPool %s/%s because server ip %s is already allocated", testIPPoolNamespace, testIPPoolName, "192.168.0.100"), + }, + }, + { + name: "invalid router ip which is malformed", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + Router("192.168.0.1000"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), + }, + }, + { + name: "invalid router ip which is out of subnet", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + Router("192.168.1.1"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because router ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.1"), + }, + }, + { + name: "invalid router ip which is the same as network ip", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + Router("192.168.0.0"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because router ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), + }, + }, + { + name: "invalid router ip which is the same as broadcast ip", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + Router("192.168.0.255"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because router ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), + }, + }, + { + name: "invalid start ip which is malformed", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + PoolRange("192.168.0.1000", ""). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), + }, + }, + { + name: "invalid start ip which is out of subnet", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + PoolRange("192.168.1.100", ""). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because start ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.100"), + }, + }, + { + name: "invalid start ip which is the same as network ip", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + PoolRange("192.168.0.0", ""). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because start ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), + }, + }, + { + name: "invalid start ip which is the same as broadcast ip", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + PoolRange("192.168.0.255", ""). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because start ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), + }, + }, + { + name: "invalid end ip which is malformed", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + PoolRange("", "192.168.0.1000"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), + }, + }, + { + name: "invalid end ip which is out of subnet", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + PoolRange("", "192.168.1.100"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because end ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.100"), + }, + }, + { + name: "invalid emd ip which is the same as network ip", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + PoolRange("", "192.168.0.0"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because end ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), + }, + }, + { + name: "invalid end ip which is the same as broadcast ip", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + PoolRange("", "192.168.0.255"). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because end ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), + }, + }, + { + name: "non-existed network name", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + NetworkName("nonexist").Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool default/net-1 because network-attachment-definitions.k8s.cni.cncf.io \"%s\" not found", "nonexist"), }, }, }