diff --git a/cmd/webhook/root.go b/cmd/webhook/root.go index f540dd6..155f98d 100644 --- a/cmd/webhook/root.go +++ b/cmd/webhook/root.go @@ -13,12 +13,15 @@ import ( "github.com/harvester/webhook/pkg/config" ) +const defaultServiceCIDR = "10.53.0.0/16" + var ( logDebug bool logTrace bool - name string - options config.Options + name string + serviceCIDR string + options config.Options ) // rootCmd represents the base command when called without any subcommands @@ -58,6 +61,8 @@ func init() { rootCmd.PersistentFlags().BoolVar(&logTrace, "trace", trace, "set logging level to trace") rootCmd.Flags().StringVar(&name, "name", os.Getenv("VM_DHCP_AGENT_NAME"), "The name of the vm-dhcp-webhook instance") + rootCmd.Flags().StringVar(&serviceCIDR, "service-cidr", defaultServiceCIDR, "The service CIDR that the cluster is currently using") + rootCmd.Flags().StringVar(&options.ControllerUsername, "controller-user", "harvester-vm-dhcp-controller", "The harvester controller username") rootCmd.Flags().StringVar(&options.GarbageCollectionUsername, "gc-user", "system:serviceaccount:kube-system:generic-garbage-collector", "The system username that performs garbage collection") rootCmd.Flags().StringVar(&options.Namespace, "namespace", os.Getenv("NAMESPACE"), "The harvester namespace") diff --git a/cmd/webhook/run.go b/cmd/webhook/run.go index af1ddcc..064d1fb 100644 --- a/cmd/webhook/run.go +++ b/cmd/webhook/run.go @@ -10,7 +10,6 @@ import ( "k8s.io/client-go/rest" ctlcore "github.com/harvester/vm-dhcp-controller/pkg/generated/controllers/core" - ctlcorev1 "github.com/harvester/vm-dhcp-controller/pkg/generated/controllers/core/v1" ctlcni "github.com/harvester/vm-dhcp-controller/pkg/generated/controllers/k8s.cni.cncf.io" ctlcniv1 "github.com/harvester/vm-dhcp-controller/pkg/generated/controllers/k8s.cni.cncf.io/v1" ctlkubevirt "github.com/harvester/vm-dhcp-controller/pkg/generated/controllers/kubevirt.io" @@ -26,9 +25,8 @@ type caches struct { ippoolCache ctlnetworkv1.IPPoolCache vmnetcfgCache ctlnetworkv1.VirtualMachineNetworkConfigCache - nadCache ctlcniv1.NetworkAttachmentDefinitionCache - nodeCache ctlcorev1.NodeCache - vmCache ctlkubevirtv1.VirtualMachineCache + nadCache ctlcniv1.NetworkAttachmentDefinitionCache + vmCache ctlkubevirtv1.VirtualMachineCache } func newCaches(ctx context.Context, cfg *rest.Config, threadiness int) (*caches, error) { @@ -51,7 +49,6 @@ func newCaches(ctx context.Context, cfg *rest.Config, threadiness int) (*caches, ippoolCache: networkFactory.Network().V1alpha1().IPPool().Cache(), vmnetcfgCache: networkFactory.Network().V1alpha1().VirtualMachineNetworkConfig().Cache(), nadCache: cniFactory.K8s().V1().NetworkAttachmentDefinition().Cache(), - nodeCache: coreFactory.Core().V1().Node().Cache(), vmCache: kubevirtFactory.Kubevirt().V1().VirtualMachine().Cache(), } @@ -76,7 +73,7 @@ func run(ctx context.Context, cfg *rest.Config, options *config.Options) error { webhookServer := server.NewWebhookServer(ctx, cfg, name, options) if err := webhookServer.RegisterValidators( - ippool.NewValidator(c.nadCache, c.nodeCache, c.vmnetcfgCache), + ippool.NewValidator(serviceCIDR, c.nadCache, c.vmnetcfgCache), vmnetcfg.NewValidator(c.ippoolCache), ); err != nil { return err diff --git a/pkg/util/common.go b/pkg/util/common.go index 72158bc..a50a734 100644 --- a/pkg/util/common.go +++ b/pkg/util/common.go @@ -10,9 +10,10 @@ const ( ExcludedMark = "EXCLUDED" ReservedMark = "RESERVED" - AgentSuffixName = "agent" - NodeArgsAnnotationKey = "rke2.io/node-args" - ServiceCIDRFlag = "--service-cidr" + AgentSuffixName = "agent" + NodeArgsAnnotationKey = "rke2.io/node-args" + ServiceCIDRFlag = "--service-cidr" + ManagementNodeLabelKey = "node-role.kubernetes.io/control-plane" ) func agentConcatName(name ...string) string { diff --git a/pkg/webhook/common.go b/pkg/webhook/common.go index 108e099..895a05d 100644 --- a/pkg/webhook/common.go +++ b/pkg/webhook/common.go @@ -1,7 +1,7 @@ package webhook const ( - CreateErr = "could not create %s %s/%s because %w" - UpdateErr = "could not update %s %s/%s because %w" - DeleteErr = "could not delete %s %s/%s because %w" + CreateErr = "cannot create %s %s/%s because %w" + UpdateErr = "cannot update %s %s/%s because %w" + DeleteErr = "cannot delete %s %s/%s because %w" ) diff --git a/pkg/webhook/ippool/mutator_test.go b/pkg/webhook/ippool/mutator_test.go index 33d3804..98cbc1d 100644 --- a/pkg/webhook/ippool/mutator_test.go +++ b/pkg/webhook/ippool/mutator_test.go @@ -194,7 +194,7 @@ func TestMutator_Create(t *testing.T) { Exclude("172.19.64.129", "172.19.64.131", "172.19.64.132", "172.19.64.133", "172.19.64.134").Build(), }, expected: output{ - err: fmt.Errorf("could not create IPPool %s/%s because fail to assign ip for dhcp server", testIPPoolNamespace, testIPPoolName), + err: fmt.Errorf("cannot create IPPool %s/%s because fail to assign ip for dhcp server", testIPPoolNamespace, testIPPoolName), }, }, { @@ -204,7 +204,7 @@ func TestMutator_Create(t *testing.T) { CIDR("172.19.64.128/32").Build(), }, expected: output{ - err: fmt.Errorf("could not create IPPool %s/%s because fail to assign ip for dhcp server", testIPPoolNamespace, testIPPoolName), + err: fmt.Errorf("cannot create IPPool %s/%s because fail to assign ip for dhcp server", testIPPoolNamespace, testIPPoolName), }, }, { @@ -214,7 +214,7 @@ func TestMutator_Create(t *testing.T) { CIDR("172.19.64.128/31").Build(), }, expected: output{ - err: fmt.Errorf("could not create IPPool %s/%s because fail to assign ip for dhcp server", testIPPoolNamespace, testIPPoolName), + err: fmt.Errorf("cannot create IPPool %s/%s because fail to assign ip for dhcp server", testIPPoolNamespace, testIPPoolName), }, }, { diff --git a/pkg/webhook/ippool/validator.go b/pkg/webhook/ippool/validator.go index 0997007..6220f87 100644 --- a/pkg/webhook/ippool/validator.go +++ b/pkg/webhook/ippool/validator.go @@ -9,11 +9,9 @@ import ( "github.com/rancher/wrangler/pkg/kv" "github.com/sirupsen/logrus" admissionregv1 "k8s.io/api/admissionregistration/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" networkv1 "github.com/harvester/vm-dhcp-controller/pkg/apis/network.harvesterhci.io/v1alpha1" - ctlcorev1 "github.com/harvester/vm-dhcp-controller/pkg/generated/controllers/core/v1" ctlcniv1 "github.com/harvester/vm-dhcp-controller/pkg/generated/controllers/k8s.cni.cncf.io/v1" ctlnetworkv1 "github.com/harvester/vm-dhcp-controller/pkg/generated/controllers/network.harvesterhci.io/v1alpha1" "github.com/harvester/vm-dhcp-controller/pkg/util" @@ -23,19 +21,20 @@ import ( type Validator struct { admission.DefaultValidator + serviceCIDR string + nadCache ctlcniv1.NetworkAttachmentDefinitionCache - nodeCache ctlcorev1.NodeCache vmnetcfgCache ctlnetworkv1.VirtualMachineNetworkConfigCache } func NewValidator( + serviceCIDR string, nadCache ctlcniv1.NetworkAttachmentDefinitionCache, - nodeCache ctlcorev1.NodeCache, vmnetcfgCache ctlnetworkv1.VirtualMachineNetworkConfigCache, ) *Validator { return &Validator{ + serviceCIDR: serviceCIDR, nadCache: nadCache, - nodeCache: nodeCache, vmnetcfgCache: vmnetcfgCache, } } @@ -161,25 +160,13 @@ func (v *Validator) checkCIDR(cidr string) error { return nil } - nodes, err := v.nodeCache.List(labels.Everything()) + svcIPNet, _, _, err := util.LoadCIDR(v.serviceCIDR) if err != nil { return err } - for _, node := range nodes { - serviceCIDR, err := util.GetServiceCIDRFromNode(node) - if err != nil { - return err - } - - svcIPNet, _, _, err := util.LoadCIDR(serviceCIDR) - if err != nil { - return err - } - - if ipNet.Contains(svcIPNet.IP) || svcIPNet.Contains(ipNet.IP) { - return fmt.Errorf("cidr %s overlaps service cidr %s", cidr, serviceCIDR) - } + if ipNet.Contains(svcIPNet.IP) || svcIPNet.Contains(ipNet.IP) { + return fmt.Errorf("cidr %s overlaps cluster service cidr %s", cidr, svcIPNet) } return nil @@ -237,15 +224,15 @@ func (v *Validator) checkServerIP(pi util.PoolInfo, unallocatables ...netip.Addr } if pi.ServerIPAddr.As4() == pi.NetworkIPAddr.As4() { - return fmt.Errorf("server ip %s cannot be the same as network ip", pi.ServerIPAddr) + return fmt.Errorf("server ip %s is the same as network ip", pi.ServerIPAddr) } if pi.ServerIPAddr.As4() == pi.BroadcastIPAddr.As4() { - return fmt.Errorf("server ip %s cannot be the same as broadcast ip", pi.ServerIPAddr) + return fmt.Errorf("server ip %s is the same as broadcast ip", pi.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) + return fmt.Errorf("server ip %s is the same as router ip", pi.ServerIPAddr) } for _, ip := range unallocatables { diff --git a/pkg/webhook/ippool/validator_test.go b/pkg/webhook/ippool/validator_test.go index 6d240bc..e44e5f2 100644 --- a/pkg/webhook/ippool/validator_test.go +++ b/pkg/webhook/ippool/validator_test.go @@ -78,7 +78,7 @@ func TestValidator_Create(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not create IPPool %s/%s because server ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, testServerIPOutOfRange), + err: fmt.Errorf("cannot create IPPool %s/%s because server ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, testServerIPOutOfRange), }, }, { @@ -91,7 +91,7 @@ func TestValidator_Create(t *testing.T) { 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("cannot create IPPool %s/%s because server ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.128"), }, }, { @@ -104,7 +104,7 @@ func TestValidator_Create(t *testing.T) { 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("cannot create IPPool %s/%s because server ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.127"), }, }, { @@ -118,7 +118,7 @@ func TestValidator_Create(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because server ip %s is the same as router ip", testIPPoolNamespace, testIPPoolName, "192.168.0.254"), }, }, { @@ -131,7 +131,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), }, }, { @@ -144,7 +144,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because router ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.1"), }, }, { @@ -157,7 +157,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because router ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), }, }, { @@ -170,7 +170,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because router ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), }, }, { @@ -183,7 +183,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), }, }, { @@ -196,7 +196,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because start ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.100"), }, }, { @@ -209,7 +209,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because start ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), }, }, { @@ -222,7 +222,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because start ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), }, }, { @@ -235,7 +235,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), }, }, { @@ -248,7 +248,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because end ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.100"), }, }, { @@ -261,7 +261,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because end ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), }, }, { @@ -274,7 +274,7 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool %s/%s because end ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), }, }, { @@ -286,11 +286,11 @@ func TestValidator_Create(t *testing.T) { 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"), + err: fmt.Errorf("cannot create IPPool default/net-1 because network-attachment-definitions.k8s.cni.cncf.io \"%s\" not found", "nonexist"), }, }, { - name: "cidr overlaps cluster's service cidr", + name: "cidr overlaps cluster's service cidr (retrieved from the node-args annotation)", given: input{ ipPool: newTestIPPoolBuilder(). CIDR(testCIDROverlap). @@ -301,12 +301,35 @@ func TestValidator_Create(t *testing.T) { Annotations: map[string]string{ util.NodeArgsAnnotationKey: fmt.Sprintf("[\"%s\", \"%s\"]", util.ServiceCIDRFlag, testServiceCIDR), }, + Labels: map[string]string{ + util.ManagementNodeLabelKey: "true", + }, Name: "node-0", }, }, }, expected: output{ - err: fmt.Errorf("could not create IPPool %s/%s because cidr %s overlaps service cidr %s", testIPPoolNamespace, testIPPoolName, testCIDROverlap, testServiceCIDR), + err: fmt.Errorf("cannot create IPPool %s/%s because cidr %s overlaps cluster service cidr %s", testIPPoolNamespace, testIPPoolName, testCIDROverlap, testServiceCIDR), + }, + }, + { + name: "cidr overlaps cluster's service cidr (default service cidr)", + given: input{ + ipPool: newTestIPPoolBuilder(). + CIDR(testCIDROverlap). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + util.ManagementNodeLabelKey: "true", + }, + Name: "node-0", + }, + }, + }, + expected: output{ + err: fmt.Errorf("cannot create IPPool %s/%s because cidr %s overlaps cluster service cidr %s", testIPPoolNamespace, testIPPoolName, testCIDROverlap, testServiceCIDR), }, }, } @@ -329,9 +352,8 @@ func TestValidator_Create(t *testing.T) { } nadCache := fakeclient.NetworkAttachmentDefinitionCache(clientset.K8sCniCncfIoV1().NetworkAttachmentDefinitions) - nodeCache := fakeclient.NodeCache(k8sclientset.CoreV1().Nodes) vmnetCache := fakeclient.VirtualMachineNetworkConfigCache(clientset.NetworkV1alpha1().VirtualMachineNetworkConfigs) - validator := NewValidator(nadCache, nodeCache, vmnetCache) + validator := NewValidator(testServiceCIDR, nadCache, vmnetCache) err = validator.Create(&admission.Request{}, tc.given.ipPool) @@ -388,7 +410,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because server ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, testServerIPOutOfRange), + err: fmt.Errorf("cannot update IPPool %s/%s because server ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, testServerIPOutOfRange), }, }, { @@ -405,7 +427,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because server ip %s cannot be the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), + err: fmt.Errorf("cannot update IPPool %s/%s because server ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), }, }, { @@ -422,7 +444,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because server ip %s cannot be the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), + err: fmt.Errorf("cannot update IPPool %s/%s because server ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), }, }, { @@ -441,7 +463,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because server ip %s cannot be the same as router ip", testIPPoolNamespace, testIPPoolName, "192.168.0.254"), + err: fmt.Errorf("cannot update IPPool %s/%s because server ip %s is the same as router ip", testIPPoolNamespace, testIPPoolName, "192.168.0.254"), }, }, { @@ -461,7 +483,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because server ip %s is already occupied", testIPPoolNamespace, testIPPoolName, "192.168.0.100"), + err: fmt.Errorf("cannot update IPPool %s/%s because server ip %s is already occupied", testIPPoolNamespace, testIPPoolName, "192.168.0.100"), }, }, { @@ -474,7 +496,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), + err: fmt.Errorf("cannot update IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), }, }, { @@ -487,7 +509,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because router ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.1"), + err: fmt.Errorf("cannot update IPPool %s/%s because router ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.1"), }, }, { @@ -500,7 +522,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because router ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), + err: fmt.Errorf("cannot update IPPool %s/%s because router ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), }, }, { @@ -513,7 +535,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because router ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), + err: fmt.Errorf("cannot update IPPool %s/%s because router ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), }, }, { @@ -526,7 +548,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), + err: fmt.Errorf("cannot update IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), }, }, { @@ -539,7 +561,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because start ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.100"), + err: fmt.Errorf("cannot update IPPool %s/%s because start ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.100"), }, }, { @@ -552,7 +574,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because start ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), + err: fmt.Errorf("cannot update IPPool %s/%s because start ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), }, }, { @@ -565,7 +587,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because start ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), + err: fmt.Errorf("cannot update IPPool %s/%s because start ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), }, }, { @@ -578,7 +600,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), + err: fmt.Errorf("cannot update IPPool %s/%s because ParseAddr(\"%s\"): IPv4 field has value >255", testIPPoolNamespace, testIPPoolName, "192.168.0.1000"), }, }, { @@ -591,7 +613,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because end ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.100"), + err: fmt.Errorf("cannot update IPPool %s/%s because end ip %s is not within subnet", testIPPoolNamespace, testIPPoolName, "192.168.1.100"), }, }, { @@ -604,7 +626,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because end ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), + err: fmt.Errorf("cannot update IPPool %s/%s because end ip %s is the same as network ip", testIPPoolNamespace, testIPPoolName, "192.168.0.0"), }, }, { @@ -617,7 +639,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because end ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), + err: fmt.Errorf("cannot update IPPool %s/%s because end ip %s is the same as broadcast ip", testIPPoolNamespace, testIPPoolName, "192.168.0.255"), }, }, { @@ -629,11 +651,11 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool default/net-1 because network-attachment-definitions.k8s.cni.cncf.io \"%s\" not found", "nonexist"), + err: fmt.Errorf("cannot update IPPool default/net-1 because network-attachment-definitions.k8s.cni.cncf.io \"%s\" not found", "nonexist"), }, }, { - name: "cidr overlaps cluster's service cidr", + name: "cidr overlaps cluster's service cidr (retrieved from the node-args annotation)", given: input{ newIPPool: newTestIPPoolBuilder(). CIDR(testCIDROverlap). @@ -644,12 +666,60 @@ func TestValidator_Update(t *testing.T) { Annotations: map[string]string{ util.NodeArgsAnnotationKey: fmt.Sprintf("[\"%s\", \"%s\"]", util.ServiceCIDRFlag, testServiceCIDR), }, + Labels: map[string]string{ + util.ManagementNodeLabelKey: "true", + }, + Name: "node-0", + }, + }, + }, + expected: output{ + err: fmt.Errorf("cannot update IPPool %s/%s because cidr %s overlaps cluster service cidr %s", testIPPoolNamespace, testIPPoolName, testCIDROverlap, testServiceCIDR), + }, + }, + { + name: "cidr overlaps cluster's service cidr (default service cidr)", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDROverlap). + NetworkName(testNetworkName).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + util.ManagementNodeLabelKey: "true", + }, Name: "node-0", }, }, }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because cidr %s overlaps service cidr %s", testIPPoolNamespace, testIPPoolName, testCIDROverlap, testServiceCIDR), + err: fmt.Errorf("cannot update IPPool %s/%s because cidr %s overlaps cluster service cidr %s", testIPPoolNamespace, testIPPoolName, testCIDROverlap, testServiceCIDR), + }, + }, + { + name: "server ip collides with excluded ip", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + ServerIP(testExcludedIP). + NetworkName(testNetworkName). + Allocated(testExcludedIP, util.ExcludedMark).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), + }, + expected: output{ + err: fmt.Errorf("cannot update IPPool %s/%s because server ip %s is already occupied", testIPPoolNamespace, testIPPoolName, testExcludedIP), + }, + }, + { + name: "server ip within the pool range", + given: input{ + newIPPool: newTestIPPoolBuilder(). + CIDR(testCIDR). + ServerIP(testServerIPWithinRange). + NetworkName(testNetworkName). + Allocated(testServerIPWithinRange, util.ReservedMark).Build(), + nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, }, { @@ -663,7 +733,7 @@ func TestValidator_Update(t *testing.T) { nad: newTestNetworkAttachmentDefinitionBuilder().Build(), }, expected: output{ - err: fmt.Errorf("could not update IPPool %s/%s because server ip %s is already occupied", testIPPoolNamespace, testIPPoolName, testExcludedIP), + err: fmt.Errorf("cannot update IPPool %s/%s because server ip %s is already occupied", testIPPoolNamespace, testIPPoolName, testExcludedIP), }, }, { @@ -697,9 +767,8 @@ func TestValidator_Update(t *testing.T) { } nadCache := fakeclient.NetworkAttachmentDefinitionCache(clientset.K8sCniCncfIoV1().NetworkAttachmentDefinitions) - nodeCache := fakeclient.NodeCache(k8sclientset.CoreV1().Nodes) vmnetCache := fakeclient.VirtualMachineNetworkConfigCache(clientset.NetworkV1alpha1().VirtualMachineNetworkConfigs) - validator := NewValidator(nadCache, nodeCache, vmnetCache) + validator := NewValidator(testServiceCIDR, nadCache, vmnetCache) err = validator.Update(&admission.Request{}, tc.given.oldIPPool, tc.given.newIPPool)