Skip to content

Commit

Permalink
fix(webhook): validate pool-related inputs
Browse files Browse the repository at this point in the history
Signed-off-by: Zespre Chang <zespre.chang@suse.com>
  • Loading branch information
starbops committed Feb 16, 2024
1 parent fd749fb commit 19f246c
Show file tree
Hide file tree
Showing 6 changed files with 536 additions and 58 deletions.
14 changes: 10 additions & 4 deletions pkg/controller/ippool/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func prepareAgentPod(
clusterNetwork string,
agentServiceAccountName string,
agentImage *config.Image,
) *corev1.Pod {
) (*corev1.Pod, error) {
name := util.SafeAgentConcatName(ipPool.Namespace, ipPool.Name)

nadNamespace, nadName := kv.RSplit(ipPool.Spec.NetworkName, "/")
Expand All @@ -37,9 +37,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{
Expand Down Expand Up @@ -143,7 +149,7 @@ func prepareAgentPod(
},
},
},
}
}, nil
}

func setRegisteredCondition(ipPool *networkv1.IPPool, status corev1.ConditionStatus, reason, message string) {
Expand Down
5 changes: 4 additions & 1 deletion pkg/controller/ippool/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,10 @@ func (h *Handler) DeployAgent(ipPool *networkv1.IPPool, status networkv1.IPPoolS
}
}

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
}

if status.AgentPodRef == nil {
status.AgentPodRef = new(networkv1.PodReference)
Expand Down
12 changes: 6 additions & 6 deletions pkg/controller/ippool/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func TestHandler_DeployAgent(t *testing.T) {

expectedStatus := newTestIPPoolStatusBuilder().
AgentPodRef(testPodNamespace, testPodName, testImage, "").Build()
expectedPod := prepareAgentPod(
expectedPod, _ := prepareAgentPod(
NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName).
ServerIP(testServerIP).
CIDR(testCIDR).
Expand Down Expand Up @@ -336,7 +336,7 @@ func TestHandler_DeployAgent(t *testing.T) {
AgentPodRef(testPodNamespace, testPodName, testImage, "").Build()
givenNAD := newTestNetworkAttachmentDefinitionBuilder().
Label(clusterNetworkLabelKey, testClusterNetwork).Build()
givenPod := prepareAgentPod(
givenPod, _ := prepareAgentPod(
NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName).
ServerIP(testServerIP).
CIDR(testCIDR).
Expand All @@ -353,7 +353,7 @@ func TestHandler_DeployAgent(t *testing.T) {

expectedStatus := newTestIPPoolStatusBuilder().
AgentPodRef(testPodNamespace, testPodName, testImage, "").Build()
expectedPod := prepareAgentPod(
expectedPod, _ := prepareAgentPod(
NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName).
ServerIP(testServerIP).
CIDR(testCIDR).
Expand Down Expand Up @@ -408,12 +408,12 @@ func TestHandler_DeployAgent(t *testing.T) {
ServerIP(testServerIP).
CIDR(testCIDR).
NetworkName(testNetworkNameLong).Build()
givenNAD := newNetworkAttachmentDefinitionBuilder(testNADNamespace, testNADNameLong).
givenNAD := NewNetworkAttachmentDefinitionBuilder(testNADNamespace, testNADNameLong).
Label(clusterNetworkLabelKey, testClusterNetwork).Build()

expectedStatus := newTestIPPoolStatusBuilder().
AgentPodRef(testPodNamespace, testPodNameLong, testImage, "").Build()
expectedPod := prepareAgentPod(
expectedPod, _ := prepareAgentPod(
NewIPPoolBuilder(testIPPoolNamespace, testIPPoolNameLong).
ServerIP(testServerIP).
CIDR(testCIDR).
Expand Down Expand Up @@ -510,7 +510,7 @@ func TestHandler_DeployAgent(t *testing.T) {
AgentPodRef(testPodNamespace, testPodName, testImage, testUID).Build()
givenNAD := newTestNetworkAttachmentDefinitionBuilder().
Label(clusterNetworkLabelKey, testClusterNetwork).Build()
givenPod := prepareAgentPod(
givenPod, _ := prepareAgentPod(
NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName).
ServerIP(testServerIP).
CIDR(testCIDR).
Expand Down
62 changes: 61 additions & 1 deletion pkg/util/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
129 changes: 100 additions & 29 deletions pkg/webhook/ippool/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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)
}

Expand Down Expand Up @@ -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"
}
Expand All @@ -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,
Expand Down
Loading

0 comments on commit 19f246c

Please sign in to comment.